1Helm 0.0.23: desktop connections and Cowork polish - #32
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe 0.0.23 release adds desktop and mobile workspace connection flows, one-time host update prompts, Cowork folder and viewport handling, custom provider route visualization, and updated channel-machine defaults with corresponding tests and documentation. ChangesDesktop workspace targeting
Mobile workspace connection
Host update prompts
Cowork context and viewport behavior
Custom provider routing
Release and provisioning defaults
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DesktopGateway
participant DesktopMain
participant CompatibilityAPI
DesktopGateway->>DesktopMain: Submit workspace or instance origin
DesktopMain->>CompatibilityAPI: Validate remote compatibility
CompatibilityAPI-->>DesktopMain: Return compatibility and setup status
DesktopMain->>DesktopGateway: Load workspace or display connection error
sequenceDiagram
participant MobileGateway
participant InstanceGateway
participant CompatibilityAPI
MobileGateway->>InstanceGateway: Read existing server origin
MobileGateway->>CompatibilityAPI: Check selected origin
CompatibilityAPI-->>MobileGateway: Return compatibility and setup status
MobileGateway->>InstanceGateway: Select server origin
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 5
🤖 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 `@desktop/main.cjs`:
- Around line 342-350: Gate createWindow() calls triggered by second-instance
and activate behind the same startup promise used by startLocalRuntime(),
ensuring loadInitialWorkspace() runs only after the runtime has initialized
localOrigin. Introduce or reuse a shared startup guard in the desktop startup
flow, while preserving the existing behavior for platforms that call
stopAutomaticServerStartup().
In `@desktop/workspace-target.cjs`:
- Around line 3-4: Expand the RESERVED_WORKSPACES set to include every
system-reserved *.1helm.com hostname defined by the backend, including the
listed admin, api, app, assets, auth, billing, blog, cdn, docs, help, mail,
security, status, support, www, 1helm, cloudflare, login, signup, and static
entries, while preserving the existing demo and provision reservations.
In `@src/client/app.ts`:
- Around line 174-182: Update scheduleHostUpdatePromptChecks and boot to
invalidate in-flight polls across boot and rescheduling: increment a shared
updateReadyPollGeneration before clearing the timeout, capture the current
generation when poll starts, and verify it before showing the update prompt and
before scheduling the next timeout. Ensure stale requests cannot process
responses or recreate the polling loop after logout or reboot.
- Around line 155-162: Update maybeShowUpdateReadyPrompt so the stored
suppression identity includes S.me.id along with the existing update and version
fields, ensuring prompt dismissal is scoped to the current admin. Add a
regression test covering two admin accounts using the same browser storage and
verify the second account still receives its prompt.
In `@src/client/cowork.ts`:
- Around line 558-565: Update the send function to capture the current Cowork
context and chatRootId before awaiting api, then persist any newly created
thread root using the captured context rather than current shared state. After
the request resolves, only clear coworkContextPending, input, drafts, and
rerender when the captured context remains active; ensure newer context state is
not overwritten.
🪄 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: ddc0c055-cc5c-4c09-bfc0-c627daa0f12e
⛔ Files ignored due to path filters (2)
mobile-gateway/1helm-logo.pngis excluded by!**/*.pngpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (26)
CHANGELOG.mdREADME.mddesktop/gateway.htmldesktop/main.cjsdesktop/workspace-target.cjsdocs/USER_GUIDE.mdmobile-gateway/error.htmlmobile-gateway/index.htmlpackage.jsonpublic/index.htmlsrc/client/app.tssrc/client/cowork-editors.tssrc/client/cowork.tssrc/client/routing.tssrc/client/styles.csssrc/server/channel-computers.tssrc/server/cowork-collaboration.tssrc/server/db.tssrc/server/index.tstest/brief-regressions-browser.mjstest/channel-computers.mjstest/channel-surfaces.mjstest/cowork-browser.mjstest/desktop.mjstest/mobile.mjstest/routing-ui-contract.mjs
Comment on lines
+342
to
+350
| const mode = desktopMode(); | ||
| if (mode === "server" || process.platform === "linux") { | ||
| keepSkipperAvailable(); | ||
| prepareWindowsWslDataRoot(); | ||
| await startLocalRuntime(); | ||
| hostUpdateService.schedule(); | ||
| } else { | ||
| stopAutomaticServerStartup(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
sed -n '330,365p' desktop/main.cjsRepository: gitcommit90/1Helm
Length of output: 1560
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== createWindow and loadInitialWorkspace definitions =="
rg -n "function createWindow|const createWindow|loadInitialWorkspace|createWindow\\(" desktop/main.cjs
echo
echo "== surrounding createWindow implementation =="
python3 - <<'PY'
from pathlib import Path
p = Path("desktop/main.cjs")
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if "function createWindow" in line or "const createWindow" in line:
start = max(1, i-25)
end = min(len(text), i+120)
for j in range(start, end+1):
print(f"{j}:{text[j-1]}")
print("\n---\n")
PYRepository: gitcommit90/1Helm
Length of output: 7132
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== all app.on registrations and startup ordering =="
rg -n "app\\.on\\(|whenReady\\(|createWindow\\(|loadInitialWorkspace\\(" desktop/main.cjsRepository: gitcommit90/1Helm
Length of output: 727
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== loadInitialWorkspace implementation =="
python3 - <<'PY'
from pathlib import Path
p = Path("desktop/main.cjs")
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if "loadInitialWorkspace" in line:
start = max(1, i-30)
end = min(len(text), i+80)
for j in range(start, end+1):
print(f"{j}:{text[j-1]}")
print("\n---\n")
PYRepository: gitcommit90/1Helm
Length of output: 10068
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== any other main-window creation paths =="
rg -n "new BrowserWindow|createWindow\\(|mainWindow" desktop/main.cjsRepository: gitcommit90/1Helm
Length of output: 934
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== runtime/origin definitions =="
rg -n "let localOrigin|const localOrigin|function startLocalRuntime|async function startLocalRuntime|desktopMode\\(" desktop/main.cjs
echo
echo "== startLocalRuntime implementation slice =="
python3 - <<'PY'
from pathlib import Path
p = Path("desktop/main.cjs")
text = p.read_text().splitlines()
for i, line in enumerate(text, 1):
if "function startLocalRuntime" in line or "async function startLocalRuntime" in line:
start = max(1, i-40)
end = min(len(text), i+160)
for j in range(start, end+1):
print(f"{j}:{text[j-1]}")
print("\n---\n")
PYRepository: gitcommit90/1Helm
Length of output: 8870
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== second-instance and activate semantics in file =="
sed -n '295,370p' desktop/main.cjs | cat -nRepository: gitcommit90/1Helm
Length of output: 4245
Gate window creation on the runtime startup promise. second-instance and activate can still reach createWindow() before await startLocalRuntime() finishes, so loadInitialWorkspace() may try to load an empty localOrigin. A shared startup guard would remove the race.
🤖 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 `@desktop/main.cjs` around lines 342 - 350, Gate createWindow() calls triggered
by second-instance and activate behind the same startup promise used by
startLocalRuntime(), ensuring loadInitialWorkspace() runs only after the runtime
has initialized localOrigin. Introduce or reuse a shared startup guard in the
desktop startup flow, while preserving the existing behavior for platforms that
call stopAutomaticServerStartup().
Comment on lines
+3
to
+4
| const WORKSPACE_HOST = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?\.1helm\.com$/i; | ||
| const RESERVED_WORKSPACES = new Set(["demo.1helm.com", "provision.1helm.com"]); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare desktop reserved-workspace list against backend reserved-slug validation
rg -n 'RESERVED_WORKSPACES|reserved' desktop/workspace-target.cjs
rg -n 'slugValid|reserved' cloudflare/src/worker.tsRepository: gitcommit90/1Helm
Length of output: 972
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the backend reserved-slug definition and nearby usage.
sed -n '1,90p' cloudflare/src/worker.tsRepository: gitcommit90/1Helm
Length of output: 4593
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find all reserved-slug definitions/usages in the repo.
rg -n 'const RESERVED|RESERVED_WORKSPACES|slugValid|reserved slug|reserved' .Repository: gitcommit90/1Helm
Length of output: 8911
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the exact backend reserved-slug definition and its surrounding context.
sed -n '36,56p' cloudflare/src/worker.tsRepository: gitcommit90/1Helm
Length of output: 1401
Expand RESERVED_WORKSPACES to match the backend reserved-slug list. cloudflare/src/worker.ts reserves more system slugs than demo/provision (for example admin, api, app, assets, auth, billing, blog, cdn, docs, help, mail, security, status, support, www, 1helm, cloudflare, login, signup, static), so the desktop host check can treat reserved *.1helm.com names as workspace targets.
🤖 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 `@desktop/workspace-target.cjs` around lines 3 - 4, Expand the
RESERVED_WORKSPACES set to include every system-reserved *.1helm.com hostname
defined by the backend, including the listed admin, api, app, assets, auth,
billing, blog, cdn, docs, help, mail, security, status, support, www, 1helm,
cloudflare, login, signup, and static entries, while preserving the existing
demo and provision reservations.
Comment on lines
+155
to
+162
| const UPDATE_READY_PROMPT_KEY = "1helm.updateReadyPrompt"; | ||
| let updateReadyPoll = 0; | ||
|
|
||
| function maybeShowUpdateReadyPrompt(update: HostUpdate): void { | ||
| if (!S.me?.is_admin || update.status !== "ready") return; | ||
| const identity = `${update.mode}:${update.current_version}->${update.version || "downloaded"}`; | ||
| if (localStorage.getItem(UPDATE_READY_PROMPT_KEY) === identity || document.querySelector("[data-update-ready-prompt]")) return; | ||
| localStorage.setItem(UPDATE_READY_PROMPT_KEY, identity); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and inspect the surrounding code.
ast-grep outline src/client/app.ts --view expanded
printf '\n--- lines around maybeShowUpdateReadyPrompt ---\n'
sed -n '130,210p' src/client/app.ts
printf '\n--- search for helper usage ---\n'
rg -n "getServerOrigin|UPDATE_READY_PROMPT_KEY|maybeShowUpdateReadyPrompt|localStorage.getItem\\(|localStorage.setItem\\(" src/client/app.ts src/client -g '!**/dist/**'Repository: gitcommit90/1Helm
Length of output: 24507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect how user identity / target context is represented elsewhere.
rg -n "S\\.me|is_admin|server origin|target|host|origin|account" src/client -g '!**/dist/**'
# Look for tests or related code paths around the update prompt.
rg -n "updateReady|update ready|Later|data-update-ready-prompt" src -g '!**/dist/**'Repository: gitcommit90/1Helm
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find the definition of HostUpdate to understand whether the prompt identity already encodes a target.
rg -n "type HostUpdate|interface HostUpdate|HostUpdate" src -g '!**/dist/**'
# Read the type if found.
file=$(rg -l "type HostUpdate|interface HostUpdate" src -g '!**/dist/**' | head -n 1 || true)
if [ -n "${file:-}" ]; then
echo "--- $file ---"
sed -n '1,220p' "$file"
fiRepository: gitcommit90/1Helm
Length of output: 14657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/client/mobile.tsRepository: gitcommit90/1Helm
Length of output: 9455
Scope the prompt suppression to the current admin. The stored identity only uses update/version data, so one admin’s “Later” choice can suppress the prompt for another admin on the same browser profile. Include S.me.id in the key and add a regression case for a second account.
🤖 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/client/app.ts` around lines 155 - 162, Update maybeShowUpdateReadyPrompt
so the stored suppression identity includes S.me.id along with the existing
update and version fields, ensuring prompt dismissal is scoped to the current
admin. Add a regression test covering two admin accounts using the same browser
storage and verify the second account still receives its prompt.
Comment on lines
+174
to
+182
| function scheduleHostUpdatePromptChecks(): void { | ||
| window.clearTimeout(updateReadyPoll); | ||
| if (!S.me?.is_admin) return; | ||
| const poll = async (): Promise<void> => { | ||
| try { maybeShowUpdateReadyPrompt(await api<HostUpdate>("/api/app/update")); } | ||
| catch { /* Profile retains the visible manual retry path. */ } | ||
| updateReadyPoll = window.setTimeout(() => { void poll(); }, 30_000); | ||
| }; | ||
| updateReadyPoll = window.setTimeout(() => { void poll(); }, 25_000); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Invalidate in-flight polls during boot and rescheduling.
boot() only clears the timeout. If poll() has already started its request, that request still schedules a new timeout when it finishes, leaving a stale or duplicate polling loop after logout/reboot and potentially processing a response under a new session.
Track a generation token or abort the request, and check it before showing the prompt and before scheduling the next timeout.
Suggested generation guard
+let updateReadyPollGeneration = 0;
function scheduleHostUpdatePromptChecks(): void {
+ const generation = ++updateReadyPollGeneration;
window.clearTimeout(updateReadyPoll);
if (!S.me?.is_admin) return;
const poll = async (): Promise<void> => {
- try { maybeShowUpdateReadyPrompt(await api<HostUpdate>("/api/app/update")); }
+ try {
+ const update = await api<HostUpdate>("/api/app/update");
+ if (generation !== updateReadyPollGeneration) return;
+ maybeShowUpdateReadyPrompt(update);
+ }
catch { /* Profile retains the visible manual retry path. */ }
+ if (generation !== updateReadyPollGeneration) return;
updateReadyPoll = window.setTimeout(() => { void poll(); }, 30_000);
};Increment updateReadyPollGeneration in boot() before clearing the timeout.
Also applies to: 283-283
🤖 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/client/app.ts` around lines 174 - 182, Update
scheduleHostUpdatePromptChecks and boot to invalidate in-flight polls across
boot and rescheduling: increment a shared updateReadyPollGeneration before
clearing the timeout, capture the current generation when poll starts, and
verify it before showing the update prompt and before scheduling the next
timeout. Ensure stale requests cannot process responses or recreate the polling
loop after logout or reboot.
Comment on lines
558
to
+565
| const send = async (): Promise<void> => { | ||
| const message = input.value.trim(); if (!message || !session.path) return; | ||
| const message = input.value.trim(); if (!message) return; | ||
| input.disabled = true; | ||
| try { | ||
| const body = chatRootId ? message : `@${channel.agent?.name || "agent"} ${message}`; | ||
| const result = await api<{ message: Message }>(`/api/channels/${channelId}/messages`, { body: { body, parentId: chatRootId || null, ...(coworkContextPending ? { coworkPath: session.path } : {}) } }); | ||
| if (!chatRootId) { chatRootId = result.message.id; localStorage.setItem(threadKey(session.path), String(chatRootId)); } | ||
| coworkContextPending = false; input.value = ""; agentDrafts.delete(session.path); await renderChatMessages(); | ||
| const result = await api<{ message: Message }>(`/api/channels/${channelId}/messages`, { body: { body, parentId: chatRootId || null, ...(coworkContextPending ? { coworkPath: context, coworkKind: session.path ? "file" : "folder" } : {}) } }); | ||
| if (!chatRootId) { chatRootId = result.message.id; localStorage.setItem(threadKey(context), String(chatRootId)); } | ||
| coworkContextPending = false; input.value = ""; agentDrafts.delete(context); await renderChatMessages(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Prevent an in-flight request from overwriting a newer Cowork context.
If the user switches folders/sections before await api(...) resolves, this response writes its old chatRootId into shared state and clears coworkContextPending. The next message in the new context can become a reply to the old context’s thread without sending its own coworkPath.
Capture the request context/root before awaiting, persist a newly created root under that captured context, and update shared UI state only if that context is still active.
🤖 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/client/cowork.ts` around lines 558 - 565, Update the send function to
capture the current Cowork context and chatRootId before awaiting api, then
persist any newly created thread root using the captured context rather than
current shared state. After the request resolves, only clear
coworkContextPending, input, drafts, and rerender when the captured context
remains active; ensure newer context state is not overwritten.
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.
Outcome
Ships the complete 0.0.23 source candidate with the requested desktop connection gateway and targeted product fixes.
Acceptance ledger
[workspace].1helm.comor a different HTTPS URL, while New User? exposes Set this PC up as a 1Helm Server to Get Started. Existing server and client installs retain their mode; Linux remains server-oriented./workspace/...folder or file context.Verification
npm run typechecknpm run buildnode --test test/desktop.mjsnode test/brief-regressions-browser.mjs— 52/52node --test test/cowork-browser.mjs— real wheel input passesnode --test test/channel-surfaces.mjsnode --test test/mobile.mjsnode test/native-world.mjs— 125/125git diff --checkA full-suite retry encountered one transient mock-provider fetch failure; the affected native-world lane then passed 125/125 standalone. The first full suite before the final update-prompt commit passed 125 native assertions and 107/109 remaining tests with two environment skips.
Summary by CodeRabbit