Fix retained Windows cold-start readiness - #54
Merged
Conversation
Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
📝 WalkthroughWalkthroughThe release updates cold-start behavior for retained channel workspaces, extends Windows server readiness to three minutes, and changes channel-machine image references and package metadata to version 0.0.36. ChangesCold-start behavior and release updates
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/server/index.ts`:
- Around line 2290-2295: Move the startChannelComputerReconciler() invocation to
execute only after server.listen() has successfully opened the HTTP listener.
Keep the existing channel workspace initialization unchanged, and ensure no
reconciler timer or OCI inspection can run before the listener is ready.
In `@test/autonomy-platform.mjs`:
- Around line 203-212: Update the regression test around ensureChannelWorkspace
to insert channel_computers metadata for the channel with backend='oci',
ensuring the OCI path is selected regardless of host platform. Replace
assert.doesNotThrow with an assertion that runtime storage remains untouched
after calling ensureChannelWorkspace(channelId, { initializeRuntimeStorage:
false }), while preserving the existing workspace root_ref assertion.
In `@test/desktop.mjs`:
- Line 122: Extend the assertions in test/desktop.mjs to verify that
waitForServer uses SERVER_READY_TIMEOUT_MS as its default timeout, not merely
that the constant is declared. Assert the waitForServer default parameter or
cover equivalent delayed-response behavior while preserving the existing
platform-specific timeout check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3deb8fcc-1ca2-49a4-83c9-b9ce099e16e8
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (11)
CHANGELOG.mdREADME.mddesktop/main.cjspackage.jsonsrc/server/agents.tssrc/server/channel-computers.tssrc/server/db.tssrc/server/index.tstest/autonomy-platform.mjstest/channel-computers.mjstest/desktop.mjs
Comment on lines
+2290
to
+2295
| // Retained OCI storage is already authoritative. In particular, a cold | ||
| // Windows login can take minutes to wake WSL, and that maintenance must | ||
| // never hold the HTTP control plane behind a synchronous startup probe. | ||
| // Provisioning, Files/Cowork access, Terminal, and the fleet reconciler | ||
| // initialize or verify runtime directories when they actually need them. | ||
| ensureChannelWorkspace(Number(channel.id), { initializeRuntimeStorage: false }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 12 \
'\b(startChannelComputerReconciler|ensureOciChannelWorkspaceDirs|ensureChannelWorkspace|spawnSync)\b' \
src/serverRepository: gitcommit90/1Helm
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- server startup ---'
sed -n '2265,2325p' src/server/index.ts
printf '%s\n' '--- reconciler symbols and call sites ---'
rg -n -C 18 \
'export function startChannelComputerReconciler|function startChannelComputerReconciler|startChannelComputerReconciler\(|ensureOciChannelWorkspaceDirs|function ensureOciChannelWorkspaceDirs|export function ensureOciChannelWorkspaceDirs' \
src/server/channel-computers.ts src/server/index.ts src/server/agents.ts
printf '%s\n' '--- related runtime-storage calls in the reconciler module ---'
rg -n -C 8 \
'wake|initialize|runtime|workspace|spawnSync|spawn\(|exec|ensure.*Dirs|reconcil' \
src/server/channel-computers.tsRepository: gitcommit90/1Helm
Length of output: 50374
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- reconciler execution path ---'
sed -n '730,835p' src/server/channel-computers.ts
sed -n '1420,1588p' src/server/channel-computers.ts
printf '%s\n' '--- connector awaited before listen ---'
rg -n -C 24 \
'function startPhotonConnector|export function startPhotonConnector|export async function startPhotonConnector|startPhotonConnector\(' \
src/server
printf '%s\n' '--- exact startup ordering ---'
python3 - <<'PY'
from pathlib import Path
p = Path("src/server/index.ts").read_text()
for needle in ("startChannelComputerReconciler();", "server.listen(", "await startPhotonConnector()"):
print(needle, p.find(needle))
PYRepository: gitcommit90/1Helm
Length of output: 33142
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
index = Path("src/server/index.ts").read_text()
computers = Path("src/server/channel-computers.ts").read_text()
startup = index[index.index("startChannelComputerReconciler();"):index.index("server.listen(", index.index("startChannelComputerReconciler();"))]
reconciler = computers[computers.index("async function reconcileOne"):computers.index("export async function reconcileChannelComputers")]
helper = computers[computers.index("function ensureWindowsOciHelperInstalled"):computers.index("/**\n * Create OCI channel workspace dirs")]
checks = {
"reconciler starts before listener": index.index("startChannelComputerReconciler();") < index.index("server.listen("),
"listener is after awaited Photon startup": index.index("await startPhotonConnector()") < index.index("server.listen("),
"first tick uses a delayed timer": "setTimeout(() => { reconcileStartupTimer = null; tick(); }, INITIAL_RECONCILE_MS)" in computers,
"reconcileOne inspects isolated computer before later provisioning": "inspection = await inspectIsolated(computer)" in reconciler and reconciler.index("inspection = await inspectIsolated(computer)") < reconciler.index("await provisionChannelComputer"),
"Windows helper installation uses synchronous WSL execution": "spawnSync(wsl, [\"--distribution\", runtime" in helper,
}
for name, value in checks.items():
print(f"{name}: {value}")
print("startup sequence:")
for token in ("startChannelComputerReconciler();", "await startPhotonConnector()", "server.listen("):
print(f" {token}: {index.index(token)}")
PYRepository: gitcommit90/1Helm
Length of output: 543
Start the reconciler after server.listen()
If Photon startup takes more than two seconds, the reconciler's initial timer can fire before the listener opens. Its OCI inspection can synchronously invoke the Windows WSL helper and wake WSL. Move startChannelComputerReconciler() after server.listen() to preserve cold-start readiness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server/index.ts` around lines 2290 - 2295, Move the
startChannelComputerReconciler() invocation to execute only after
server.listen() has successfully opened the HTTP listener. Keep the existing
channel workspace initialization unchanged, and ensure no reconciler timer or
OCI inspection can run before the listener is ready.
Comment on lines
+203
to
+212
| test("retained OCI channel metadata can initialize without synchronously touching runtime storage", () => { | ||
| seed(); | ||
| const channelId = run("INSERT INTO channels (name,slug,kind,topic,purpose,status,created) VALUES ('retained-cold-start','retained-cold-start','channel','','','active',?)", now()).lastInsertRowid; | ||
| const botId = run("INSERT INTO bots (name,model,prompt,created) VALUES ('retained-cold-agent','mock','Resident.',?)", now()).lastInsertRowid; | ||
| const agentId = run("INSERT INTO agents (bot_id,kind,name,status,created) VALUES (?,'channel','retained-cold-agent','ready',?)", botId, now()).lastInsertRowid; | ||
| run("INSERT INTO agent_channels (agent_id,channel_id,bound_at) VALUES (?,?,?)", agentId, channelId, now()); | ||
| run("INSERT INTO agent_profiles (agent_id,purpose,instructions,updated) VALUES (?,'Cold start','Resident.',?)", agentId, now()); | ||
| assert.doesNotThrow(() => agents.ensureChannelWorkspace(channelId, { initializeRuntimeStorage: false })); | ||
| assert.equal(q1("SELECT root_ref FROM channel_workspaces WHERE channel_id=?", channelId).root_ref, `channels/${channelId}`); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
printf '%s\n' 'Relevant files:'
git ls-files | rg '(^|/)(src/server/agents\.ts|test/autonomy-platform\.mjs)$'
printf '%s\n' 'Relevant symbols and references:'
rg -n -C 8 'ensureChannelWorkspace|channelUsesRuntimeStorage|initializeRuntimeStorage|channel-computer|runtime storage|OCI' src/server/agents.ts test/autonomy-platform.mjsRepository: gitcommit90/1Helm
Length of output: 35958
🏁 Script executed:
git ls-files | rg '(^|/)(src/server/agents\.ts|test/autonomy-platform\.mjs)$'
rg -n -C 8 'ensureChannelWorkspace|channelUsesRuntimeStorage|initializeRuntimeStorage|channel-computer|runtime storage|OCI' src/server/agents.ts test/autonomy-platform.mjsRepository: gitcommit90/1Helm
Length of output: 35909
🏁 Script executed:
printf '%s\n' 'Channel storage implementation:'
ast-grep outline src/server/channel-storage.ts
printf '%s\n' 'Channel computer implementation:'
ast-grep outline src/server/channel-computers.ts
printf '%s\n' 'Relevant definitions and schema references:'
rg -n -C 12 'function channelUsesRuntimeStorage|export .*channelUsesRuntimeStorage|channel_computers|runtime_storage|oci|computer' src/server/channel-storage.ts src/server/channel-computers.ts src/server/db.ts test/autonomy-platform.mjsRepository: gitcommit90/1Helm
Length of output: 50374
🏁 Script executed:
printf '%s\n' 'Storage and initialization logic:'
sed -n '1,90p' src/server/channel-storage.ts
sed -n '200,235p' src/server/channel-computers.ts
sed -n '60,84p' src/server/agents.ts
printf '%s\n' 'Test setup and seed definition:'
rg -n -C 15 'function seed|export function seed|HELM_CHANNEL_COMPUTER_BACKEND|process\.platform|autonomy-platform' src/server/db.ts test/autonomy-platform.mjs package.json
printf '%s\n' 'Test file metadata:'
sed -n '1,22p' test/autonomy-platform.mjsRepository: gitcommit90/1Helm
Length of output: 13806
🏁 Script executed:
printf '%s\n' 'OCI directory initialization and test-observable effects:'
sed -n '350,430p' src/server/channel-computers.ts
sed -n '430,525p' src/server/channel-computers.ts
printf '%s\n' 'Exports usable by a regression test:'
rg -n '^export (async )?(function|const)|^export \{' src/server/channel-computers.ts src/server/channel-storage.ts | rg 'ensureOci|channelUsesRuntime|channelComputer|configuredChannelBackend'Repository: gitcommit90/1Helm
Length of output: 9992
Make the regression test select the OCI path explicitly.
Insert channel_computers metadata with backend='oci'. Without this row, the test uses the host default and selects the Apple path on macOS.
Assert that runtime storage remains untouched after ensureChannelWorkspace(channelId, { initializeRuntimeStorage: false }). assert.doesNotThrow is insufficient because ensureChannelWorkspace catches runtime initialization errors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/autonomy-platform.mjs` around lines 203 - 212, Update the regression
test around ensureChannelWorkspace to insert channel_computers metadata for the
channel with backend='oci', ensuring the OCI path is selected regardless of host
platform. Replace assert.doesNotThrow with an assertion that runtime storage
remains untouched after calling ensureChannelWorkspace(channelId, {
initializeRuntimeStorage: false }), while preserving the existing workspace
root_ref assertion.
| assert.match(source, /handleSquirrelEvent/); | ||
| assert.match(source, /setAppUserModelId\("com\.squirrel\.1Helm\.1Helm"\)/, "Windows uses Squirrel's stable taskbar identity"); | ||
| assert.match(source, /DATA_NAMESPACE = "1Helm-OCI-v1"/, "the clean-slate build uses a fresh durable application-data namespace"); | ||
| assert.match(source, /SERVER_READY_TIMEOUT_MS = process\.platform === "win32" \? 3 \* 60_000 : 30_000/, "Windows gives a bounded cold WSL host enough time without weakening other desktop startup budgets"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Assert that waitForServer uses the timeout constant.
The current assertion verifies only the declaration. A regression to waitForServer(origin, timeoutMs = 30_000) would still pass.
Add an assertion for the default parameter or test the delayed-response behavior.
This assessment compares the assertion with the supplied desktop/main.cjs implementation.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/desktop.mjs` at line 122, Extend the assertions in test/desktop.mjs to
verify that waitForServer uses SERVER_READY_TIMEOUT_MS as its default timeout,
not merely that the constant is declared. Assert the waitForServer default
parameter or cover equivalent delayed-response behavior while preserving the
existing platform-specific timeout check.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Acceptance ledger
Verification
npm run typechecknpm run buildnpm run test:onboarding-browser(20 passed)npm test(native-world 127 passed; complete Node suite 117 tests, 115 passed + 2 expected Chrome-unavailable skips)git diff --checkSummary by CodeRabbit
Bug Fixes
Updates