Skip to content

Release 0.0.21: Cowork fixes and synchronized desktop train - #30

Merged
gitcommit90 merged 7 commits into
mainfrom
fix/cowork-files-real-verification
Jul 27, 2026
Merged

Release 0.0.21: Cowork fixes and synchronized desktop train#30
gitcommit90 merged 7 commits into
mainfrom
fix/cowork-files-real-verification

Conversation

@gitcommit90

@gitcommit90 gitcommit90 commented Jul 27, 2026

Copy link
Copy Markdown
Owner

1Helm 0.0.21

This release fixes the reported Cowork continuity, navigation, presentation,
editor, and Files problems and restores one synchronized release line across
macOS, Linux, and Windows. It is staged for review and has not been published.

What changed

  1. Cowork agent identity — Skipper uses the product avatar in Cowork.
  2. Cowork dictation — Notes, Docs, and agent inputs expose mic controls,
    and bare Option/Alt reaches the focused control even when the desktop shell
    would otherwise swallow the corresponding key event.
  3. Scrollable Notes — long Notes scroll through CodeMirror's real viewport
    in Write mode.
  4. No duplicate text after navigation — leaving Cowork, switching sections,
    or opening another file destroys the outgoing Yjs document and reloads the
    authoritative workspace file into a fresh document.
  5. Responsive Files — directory, tree, text, and content reads paint from
    the host mirror immediately while one coalesced refresh runs in the
    background.
  6. Predictable new-file names — Cowork prompts begin blank, preserve an
    explicit extension, and otherwise add the current tab's .md,
    .whiteboard.json, .txt, or .slides.json extension.
  7. Usable presentation menu — the Excalidraw hamburger menu escapes the
    clipped editor area, remains bounded to the visible stage, and accepts
    pointer input.
  8. Readable light-mode code editor — Cowork Code uses a light surface and
    dark text in light mode while retaining its navy dark-mode treatment.
  9. Simpler Files navigation/workspace and the five Cowork folders stay
    first; remaining root entries appear under a frontend-only expandable
    Other group.
  10. Markdown to DOCX — selecting a Markdown file exposes authenticated
    Download - DOCX export with contained Office Open XML output.
  11. More desktop workspace room — the global channel sidebar collapses to
    a compact icon rail and remembers the profile-bound preference without
    changing the mobile drawer.
  12. Synchronized platform releases — release governance now requires one
    version and one source candidate for macOS, Linux, and Windows before the
    tag or GitHub Release can be published. A missing platform pauses the
    entire release.

Additional changes

  • Service-worker offline fallbacks always return a valid Response instead of
    rejecting the FetchEvent promise.
  • Windows Squirrel packaging uses a short scratch path and excludes the
    already-browser-bundled Excalidraw source tree, avoiding legacy NuGet path
    limits without removing runtime code.
  • Windows artifacts are intentionally unsigned. NotSigned is accepted for
    1Helm until a trusted Windows signing identity is adopted; this is disclosed
    release status, not a release blocker.

Downloads and integrity

Artifact SHA-256
1Helm-0.0.21-arm64.dmg 027bef223b9a929394c7c9768418a0c512e81689f61e8a8f3c2fb313da547cfa
1Helm-0.0.21-mac-arm64.zip 5711d799aed4d1ff901239230954220dab9985de9886eb3685f4dad2f5083369
1Helm-0.0.21-linux-node.tgz 36413878f62ad5df19dba0676fd6f8241200dfa1c7ad11d5aa10183d7456d5c0
1Helm-0.0.21-windows-x64-setup.exe 1f229d3704e266311f670a5172077fc02f7324f767cc968725a57e2553981ae5
1Helm-0.0.21-full.nupkg ef3b90c11bff7fcf128b73808bc50b0f6734eb3e0d1a02711a756ce4489072c2
RELEASES b32549b7b9736a86aec15f634727a20e6471e77ffa67df906c370840e88a08e2

Runtime candidate source commit:
6eab721eb11a4ace3d28b39dd4ebcc505b71422f

Release-documentation candidate HEAD:
71683518312f78484b89b9f06c79703983935dcc

The three documentation-only commits after the runtime candidate record the
accepted unsigned-Windows policy; they do not change packaged runtime bytes.
The eventual release tag must point to the final merged commit after review.

Verification

  • Shared source verification passed TypeScript typechecking, the production
    build, the 117-assertion native-world suite, focused Cowork browser coverage,
    Files latency and channel-surface regressions, desktop packaging contracts,
    release-governance contracts, and diff whitespace checks.
  • macOS: the arm64 app, updater ZIP app, DMG, and mounted app passed strict
    Developer ID verification, Apple notarization, stapling, and Gatekeeper. The
    exact DMG was installed as /Applications/1Helm.app on the retained Mac
    Studio; version 0.0.21, bundle com.gitcommit90.1helm, arm64 process,
    configured loopback workspace, and Application Support inode 34829724
    were confirmed. App notarization submission
    5aac5b2e-313a-42b7-b1cd-e59fba21888a and DMG submission
    9d527e32-ba41-4fd2-8082-56fa0ff91f07 were accepted.
  • Linux: a retained Ubuntu 24.04 x64 system completed a staged 0.0.8 to
    0.0.21 update and a deliberate unhealthy-candidate rollback. The current
    0.0.21 systemd service, updater path unit, loopback endpoint, release
    symlink, and preserved /var/lib/1helm inode 1179777 were reconfirmed.
  • Windows: native typechecking and desktop packaging tests passed. A retained
    Windows installation completed the Squirrel 0.0.8 to 0.0.21 update and
    Setup reinstall; the 0.0.21 process and loopback endpoint were healthy,
    %APPDATA%\\1Helm retained its database and provider state, and the retained
    channel WSL distro kept /workspace while excluding /mnt/c.
  • Windows Authenticode status: both Setup and packaged app are NotSigned.
    This is intentionally accepted and must remain plainly disclosed.
  • The Mac full contract run had one Cowork startup timeout; the exact focused
    Cowork browser test passed on rerun. The Mac onboarding run completed all 20
    product assertions, then hit the known cleanup-only ENOTEMPTY while
    removing a throwaway Mnemosyne virtual environment. Neither caveat is hidden
    as an all-green run.
  • No public-download verification, tagged-commit CI, feed verification, or
    published-asset digest comparison has occurred because this candidate is
    deliberately paused before merge and release.

Licensing

1Helm 0.0.21 remains licensed under AGPL-3.0-only.


Before publication, replace the candidate/source distinction with the final
merged SHA where required, compare every artifact digest to the uploaded
GitHub asset, run required CI on the exact tag, and install the public download
on the retained platform hosts.

Summary by CodeRabbit

  • New Features

    • Added DOCX downloads for Markdown files.
    • Added collapsible desktop sidebar with saved preference.
    • Improved Files organization with core folders and an expandable “Other” section.
    • Improved dictation shortcuts and microphone focus behavior.
  • Bug Fixes

    • Improved offline fallback behavior.
    • Improved workspace file refresh responsiveness.
    • Improved editor synchronization, cleanup, scrolling, and presentation menu positioning.
  • Documentation

    • Updated release guidance to require synchronized Mac, Linux, and Windows desktop artifacts.

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Version 0.0.21 adds synchronized desktop release gates and updates workspace behavior, including Cowork editor lifecycle changes, Files mirror refresh handling, Markdown-to-DOCX export, persisted sidebar state, dictation fallback behavior, offline responses, and Windows packaging adjustments.

Changes

Release train

Layer / File(s) Summary
Synchronized desktop release policy
docs/GOVERNANCE.md, docs/VISION.md, docs/release-checklist.md, docs/release-lifecycle.md
Release documentation now requires Mac, Linux, and Windows artifacts from one version and source commit, with complete verification before publication.
Desktop artifact acceptance
docs/release-checklist.md, docs/release-lifecycle.md, docs/release-notes-template.md
Required artifacts, digests, updater checks, Windows package consistency, and publication blockers are documented.
Version and release metadata
CHANGELOG.md, README.md, package.json, src/server/channel-computers.ts, src/server/db.ts, test/channel-computers.mjs
Project metadata and channel machine image references advance to 0.0.21.
Windows packaging constraints
scripts/package-windows.cjs, test/desktop.mjs
Windows packaging excludes specified client modules and uses a short Squirrel staging path.
Release governance validation
test/release-governance.mjs
Tests verify cross-platform release gating and Windows signature rules.

Workspace experience and file workflows

Layer / File(s) Summary
Editor lifecycle and navigation
src/client/cowork.ts, src/client/styles.css
Cowork editor disposal, synchronization, file naming, scrolling, and presentation layout behavior are updated.
Sidebar and dictation controls
src/client/app.ts, src/client/styles.css, test/workspace-interactions.mjs, test/cowork-browser.mjs
Desktop sidebar collapse state is persisted, and Alt dictation tracks the associated mic control with a fallback timer.
Files tree and mirror refresh
src/client/channel.ts, src/server/channel-computers.ts, src/server/index.ts, test/files-latency.mjs
Files now groups core and other roots, refreshes mirrors with request coalescing, and avoids unnecessary read-time refreshes.
Markdown DOCX export
src/server/docx.ts, src/server/index.ts, src/client/channel.ts, package.json, test/sweep-server-integration.mjs
Markdown files can be converted and downloaded as DOCX with authorization, content, and error coverage.
Offline fallback and asset loading
public/sw.js, public/index.html, test/workspace-interactions.mjs
Offline navigation returns cached content or a 503 response, while failed uncached requests return Response.error().
Cowork browser coverage
test/cowork-browser.mjs
Browser coverage expands for dictation, collaboration reloads, scrolling, contrast, file creation, and presentation menus.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Browser
  participant FilesRoute
  participant WorkspaceStore
  participant MarkdownToDocx
  Browser->>FilesRoute: Request DOCX for a Markdown file
  FilesRoute->>WorkspaceStore: Read workspace Markdown
  WorkspaceStore-->>FilesRoute: Return Markdown content
  FilesRoute->>MarkdownToDocx: Convert Markdown
  MarkdownToDocx-->>FilesRoute: Return DOCX buffer
  FilesRoute-->>Browser: Download DOCX response
Loading
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 29.41% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description covers the changes, but it does not follow the required template sections and checklist format. Add the missing template sections: Type of change, Release notes bullets, numbered acceptance ledger with evidence, verification checklist, and post-merge items.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately captures the main release theme and synchronized desktop train changes.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/cowork-files-real-verification

Comment @coderabbitai help to get the list of available commands.

@gitcommit90
gitcommit90 merged commit 52c3b42 into main Jul 27, 2026
5 of 6 checks passed
@gitcommit90
gitcommit90 deleted the fix/cowork-files-real-verification branch July 27, 2026 23:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 9

🧹 Nitpick comments (4)
src/client/cowork.ts (1)

149-156: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value

disposeEditor clears the global speech target unconditionally.

setFocusedSpeechTarget(null) wipes whatever target is focused, including the Cowork agent textarea (which sets itself as target at Line 453). Disposing an editor via rename/move/delete while the agent input is focused silently disables bare-Alt dictation there until refocus. Consider only clearing when the current target belongs to this editor.

🤖 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 149 - 156, Update disposeEditor so it
clears the focused speech target only when the current target belongs to the
SectionSession being disposed, preserving unrelated targets such as the Cowork
agent textarea. Keep the existing editor cleanup behavior unchanged.
test/cowork-browser.mjs (2)

436-436: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Hardcoded /tmp screenshot path.

Use os.tmpdir() (or a repo-relative artifacts dir) so the suite stays portable across the synchronized Mac/Linux/Windows train.

🤖 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/cowork-browser.mjs` at line 436, Update the screenshot call near
page.screenshot to avoid the hardcoded “/tmp” path; construct the output path
using os.tmpdir() or the repository’s existing portable artifacts-directory
convention, while preserving the current filename.

337-349: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Contrast check silently assumes an opaque editor background.

getComputedStyle(editor).backgroundColor returns rgba(0, 0, 0, 0) when the wrapper is transparent; parse then yields black and the ratio is computed against a color that is never painted, producing a confusing pass/fail. Assert the alpha channel (or walk up to the first opaque ancestor) before computing.

🤖 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/cowork-browser.mjs` around lines 337 - 349, Update the contrast check in
the codeContrast page evaluation to handle transparent editor backgrounds:
either resolve the first opaque ancestor background or explicitly assert that
the parsed background alpha is opaque before calculating luminance. Ensure the
ratio is never computed from the transparent rgba(0, 0, 0, 0) wrapper color.
test/release-governance.mjs (1)

50-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Check release documents independently.

assert.match(checklist + notes, ...) allows one document to satisfy a requirement missing from the other. A required checklist artifact could be removed while this test still passes because the template contains the same name. Assert the required artifacts separately for each document that owns the contract.

🤖 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/release-governance.mjs` around lines 50 - 53, Update the
artifact-validation loop in release-governance.mjs to check the checklist and
notes documents independently rather than matching against their concatenation.
Assert each required artifact against the specific document that owns its
contract, preserving the existing artifact list and case-insensitive matching.
🤖 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 `@docs/release-checklist.md`:
- Around line 104-112: Move the git tag and gh release create commands from Step
5 to after Section 8 and all desktop-platform, matrix, and digest verification
checks. Keep release creation as a draft initially, then verify the uploaded
artifact digests before adding the publication step.

In `@docs/release-lifecycle.md`:
- Around line 119-125: Update the release procedure’s earlier step 4 to defer
tag creation until after the complete desktop artifact verification gate in step
6, preserving the requirement that all macOS, Linux, and Windows artifacts are
verified from the exact merged commit before tagging or publishing.

In `@README.md`:
- Around line 137-140: Update the release-version statement in the README to
replace “signed Windows Setup/Squirrel feed” with “Windows Setup/Squirrel feed
with disclosed signature status,” matching the signing-status policy in the
referenced governance and release checklist documentation.

In `@src/client/app.ts`:
- Around line 962-967: Add an accessible name to the Feedback button in the
collapsed sidebar control by setting its aria-label to “Open feedback”;
optionally add a matching title while preserving the existing click behavior and
collapsed/expanded content.
- Around line 975-978: Serialize persistence in toggleDesktopSidebar so rapid
toggles cannot leave concurrent PATCH requests racing; queue or debounce the
/api/me/ui-state writes and persist only the latest desktop_sidebar_collapsed
value, while keeping the immediate local toggle and renderSidebar behavior
unchanged.

In `@src/client/channel.ts`:
- Around line 612-620: Update refreshMirror so refreshDirectories() completes
and repopulates directoryCache before calling load(), rather than invoking both
concurrently after clearing the cache. Preserve the existing mirror refresh
status, error handling, and refresh coalescing behavior while ensuring only one
directory request is made per refresh.
- Line 590: Update the load handling around rootEntries so it is seeded from the
first successful result.files response regardless of requestedPath, while
preserving the existing root-path behavior and avoiding later non-root loads
overwriting the initial root entry set. Ensure drawTree can compute
otherRootFiles immediately for non-root initial paths.

In `@src/server/channel-computers.ts`:
- Around line 789-795: Update the refresh flow in
prepareChannelWorkspaceArtifact to check workspaceMirrorRefreshes for an
existing promise and await it before applying the computer-running state guard.
Preserve the current deduplication and cleanup behavior for new refreshes, while
ensuring callers arriving after shutdown still wait for any in-flight
syncGuestToHost operation.

In `@src/server/db.ts`:
- Line 1008: Update the initialization around image and the channel_computers
INSERT OR IGNORE flow to migrate persisted 0.0.20 default pins to 0.0.21,
including rows whose backend is lxc or wsl. Apply this migration only to the old
default value, while preserving any explicit HELM_CHANNEL_MACHINE_IMAGE
override. Add coverage for an existing workspace containing the old image pin.

---

Nitpick comments:
In `@src/client/cowork.ts`:
- Around line 149-156: Update disposeEditor so it clears the focused speech
target only when the current target belongs to the SectionSession being
disposed, preserving unrelated targets such as the Cowork agent textarea. Keep
the existing editor cleanup behavior unchanged.

In `@test/cowork-browser.mjs`:
- Line 436: Update the screenshot call near page.screenshot to avoid the
hardcoded “/tmp” path; construct the output path using os.tmpdir() or the
repository’s existing portable artifacts-directory convention, while preserving
the current filename.
- Around line 337-349: Update the contrast check in the codeContrast page
evaluation to handle transparent editor backgrounds: either resolve the first
opaque ancestor background or explicitly assert that the parsed background alpha
is opaque before calculating luminance. Ensure the ratio is never computed from
the transparent rgba(0, 0, 0, 0) wrapper color.

In `@test/release-governance.mjs`:
- Around line 50-53: Update the artifact-validation loop in
release-governance.mjs to check the checklist and notes documents independently
rather than matching against their concatenation. Assert each required artifact
against the specific document that owns its contract, preserving the existing
artifact list and case-insensitive matching.
🪄 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: 8ad5c7da-f30f-4bc1-a83e-8d273ca0a295

📥 Commits

Reviewing files that changed from the base of the PR and between 0318ce3 and 7168351.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (29)
  • CHANGELOG.md
  • README.md
  • docs/GOVERNANCE.md
  • docs/VISION.md
  • docs/release-checklist.md
  • docs/release-lifecycle.md
  • docs/release-notes-template.md
  • package.json
  • public/index.html
  • public/sw.js
  • scripts/package-windows.cjs
  • scripts/run-test-suite.mjs
  • src/client/app.ts
  • src/client/channel.ts
  • src/client/cowork.ts
  • src/client/styles.css
  • src/server/channel-computers.ts
  • src/server/db.ts
  • src/server/docx.ts
  • src/server/index.ts
  • test/channel-computers.mjs
  • test/channel-surfaces.mjs
  • test/cowork-browser.mjs
  • test/desktop.mjs
  • test/fake-container.mjs
  • test/files-latency.mjs
  • test/release-governance.mjs
  • test/sweep-server-integration.mjs
  • test/workspace-interactions.mjs

Comment thread docs/release-checklist.md
Comment on lines +104 to +112
Only after Sections 6–8 pass for all three desktop platforms:

```bash
git tag -a "v${VERSION}" "$MERGED_COMMIT" -m "1Helm ${VERSION}"
git push origin "refs/tags/v${VERSION}"
gh release create "v${VERSION}" \
"$DMG" "$UPDATE_ZIP" "$HEADLESS" \
"$WINDOWS_SETUP" "$WINDOWS_NUPKG" "$WINDOWS_RELEASES" \
--title "1Helm ${VERSION}" --notes-file "$RELEASE_NOTES" --draft

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Move tag and release creation after the verification sections.

These commands are still in Step 5, before Sections 6–8 run. That contradicts the hard gate on Lines 9-13; --draft still creates the GitHub Release object, and the tag is pushed before the later matrix and digest checks on Lines 176-178. Move them after Section 8, then verify uploaded digests before publication.

🤖 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 `@docs/release-checklist.md` around lines 104 - 112, Move the git tag and gh
release create commands from Step 5 to after Section 8 and all desktop-platform,
matrix, and digest verification checks. Keep release creation as a draft
initially, then verify the uploaded artifact digests before adding the
publication step.

Comment thread docs/release-lifecycle.md
Comment on lines +119 to +125
6. Before creating the tag or GitHub Release, finish the complete desktop
matrix from the exact merged commit: verified macOS DMG + updater ZIP,
Linux host archive, and Windows Setup + full `.nupkg` + literal `RELEASES`
manifest. Record whether Windows artifacts are trusted-signed or unsigned;
unsigned is accepted until 1Helm adopts a trusted Windows signing identity.
7. Publish those desktop artifacts and complete release notes together through
one GitHub Release. Never publish a subset or attach a platform later to a

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Move tagging after the desktop verification gate.

Section 6 step 4 (Lines 113-114) currently tells operators to tag after merge, while this step requires the complete Mac/Linux/Windows matrix before creating the tag. Reorder or rewrite step 4 so the numbered procedure cannot produce a tag before verification completes.

🤖 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 `@docs/release-lifecycle.md` around lines 119 - 125, Update the release
procedure’s earlier step 4 to defer tag creation until after the complete
desktop artifact verification gate in step 6, preserving the requirement that
all macOS, Linux, and Windows artifacts are verified from the exact merged
commit before tagging or publishing.

Comment thread README.md
Comment on lines +137 to +140
Mac, Linux, and Windows use one synchronized desktop release version. A release
is held in full until the DMG/updater ZIP, Linux host archive, and signed
Windows Setup/Squirrel feed have all passed native install and update
acceptance from the same source commit.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Do not promise a signed Windows artifact here.

The README still describes a “signed Windows Setup/Squirrel feed,” while docs/GOVERNANCE.md Lines 63-66 and docs/release-checklist.md Lines 98-101 explicitly allow NotSigned until trusted signing is adopted. Change this to “Windows Setup/Squirrel feed with disclosed signature status.”

🤖 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 `@README.md` around lines 137 - 140, Update the release-version statement in
the README to replace “signed Windows Setup/Squirrel feed” with “Windows
Setup/Squirrel feed with disclosed signature status,” matching the
signing-status policy in the referenced governance and release checklist
documentation.

Comment thread src/client/app.ts
Comment on lines 962 to +967
h("button", {
class: "mx-2 mb-1 flex min-h-10 items-center gap-2 rounded-md px-2 py-1.5 text-xs text-sidebar-muted hover:bg-sidebar-hover hover:text-white",
class: `${collapsed ? "mx-auto w-10 justify-center" : "mx-2"} mb-1 flex min-h-10 items-center gap-2 rounded-md px-2 py-1.5 text-xs text-sidebar-muted hover:bg-sidebar-hover hover:text-white`,
type: "button",
dataset: { feedbackAction: "" },
onclick: () => { closeMobileMenu(); openFeedback(); },
}, icon("chat", 14), "Feedback"),
h("div", { class: "flex items-center gap-1 border-t border-white/10 p-1.5" },
h("button", { class: "flex min-w-0 flex-1 items-center gap-2 rounded-md px-1.5 py-1 text-left hover:bg-sidebar-hover", title: "Open profile", onclick: (event: MouseEvent) => { closeMobileMenu(); openProfile(event.currentTarget as HTMLElement); } },
}, icon("chat", 14), collapsed ? null : "Feedback"),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Add an accessible name to the collapsed Feedback control.

When collapsed, this button contains only an unlabeled SVG, so assistive technology exposes an unnamed button. Add aria-label="Open feedback" (and optionally a matching title).

🤖 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 962 - 967, Add an accessible name to the
Feedback button in the collapsed sidebar control by setting its aria-label to
“Open feedback”; optionally add a matching title while preserving the existing
click behavior and collapsed/expanded content.

Comment thread src/client/app.ts
Comment on lines +975 to +978
function toggleDesktopSidebar(): void {
S.desktopSidebarCollapsed = !S.desktopSidebarCollapsed;
renderSidebar();
void api("/api/me/ui-state", { method: "PATCH", body: { key: "desktop_sidebar_collapsed", value: S.desktopSidebarCollapsed } }).catch(() => undefined);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Serialize preference writes.

Rapid toggles create concurrent PATCH requests. If they complete out of order, the server can retain the previous state and restore the wrong rail width after reload. Queue or debounce writes so only the final value is persisted.

Proposed fix
+let desktopSidebarPreferenceWrite: Promise<void> = Promise.resolve();
+
 function toggleDesktopSidebar(): void {
   S.desktopSidebarCollapsed = !S.desktopSidebarCollapsed;
   renderSidebar();
-  void api("/api/me/ui-state", { method: "PATCH", body: { key: "desktop_sidebar_collapsed", value: S.desktopSidebarCollapsed } }).catch(() => undefined);
+  const value = S.desktopSidebarCollapsed;
+  desktopSidebarPreferenceWrite = desktopSidebarPreferenceWrite
+    .catch(() => undefined)
+    .then(() => api("/api/me/ui-state", { method: "PATCH", body: { key: "desktop_sidebar_collapsed", value } }))
+    .then(() => undefined)
+    .catch(() => undefined);
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function toggleDesktopSidebar(): void {
S.desktopSidebarCollapsed = !S.desktopSidebarCollapsed;
renderSidebar();
void api("/api/me/ui-state", { method: "PATCH", body: { key: "desktop_sidebar_collapsed", value: S.desktopSidebarCollapsed } }).catch(() => undefined);
let desktopSidebarPreferenceWrite: Promise<void> = Promise.resolve();
function toggleDesktopSidebar(): void {
S.desktopSidebarCollapsed = !S.desktopSidebarCollapsed;
renderSidebar();
const value = S.desktopSidebarCollapsed;
desktopSidebarPreferenceWrite = desktopSidebarPreferenceWrite
.catch(() => undefined)
.then(() => api("/api/me/ui-state", { method: "PATCH", body: { key: "desktop_sidebar_collapsed", value } }))
.then(() => undefined)
.catch(() => undefined);
}
🤖 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 975 - 978, Serialize persistence in
toggleDesktopSidebar so rapid toggles cannot leave concurrent PATCH requests
racing; queue or debounce the /api/me/ui-state writes and persist only the
latest desktop_sidebar_collapsed value, while keeping the immediate local toggle
and renderSidebar behavior unchanged.

Comment thread src/client/channel.ts
try {
const result = await api<{ path?: string; files: ChannelFile[] }>(`/api/channels/${channelId}/files?path=${encodeURIComponent(requestedPath)}`);
if (requestedPath !== currentPath) return;
if (!requestedPath) rootEntries = result.files;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

rootEntries only populates from a root-path load().

rootEntries (used to compute otherRootFiles in drawTree) is set only if (!requestedPath). When renderFiles is constructed with a non-root initialPath (e.g. deep-linking into a Cowork file), the browser never issues a root-path request unless the user manually navigates back to /workspace, so the "Other" section can silently miss real root-level files until then.

💡 Possible fix: always seed root entries once
   const load = async (options: { refreshTree?: boolean } = {}): Promise<void> => {
     const requestedPath = currentPath;
     try {
       const result = await api<{ path?: string; files: ChannelFile[] }>(`/api/channels/${channelId}/files?path=${encodeURIComponent(requestedPath)}`);
       if (requestedPath !== currentPath) return;
       if (!requestedPath) rootEntries = result.files;
+      else if (!rootEntries.length) void api<{ files: ChannelFile[] }>(`/api/channels/${channelId}/files?path=`).then((r) => { rootEntries = r.files; drawTree(directoryCache || []); }).catch(() => {});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!requestedPath) rootEntries = result.files;
if (!requestedPath) rootEntries = result.files;
else if (!rootEntries.length) void api<{ files: ChannelFile[] }>(`/api/channels/${channelId}/files?path=`).then((r) => { rootEntries = r.files; drawTree(directoryCache || []); }).catch(() => {});
🤖 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/channel.ts` at line 590, Update the load handling around
rootEntries so it is seeded from the first successful result.files response
regardless of requestedPath, while preserving the existing root-path behavior
and avoiding later non-root loads overwriting the initial root entry set. Ensure
drawTree can compute otherRootFiles immediately for non-root initial paths.

Comment thread src/client/channel.ts
Comment on lines +612 to +620
const refreshMirror = (): Promise<void> => {
if (mirrorRefresh) return mirrorRefresh;
status.textContent = "Refreshing from the channel computer…";
mirrorRefresh = api(`/api/channels/${channelId}/files/refresh`, { method: "POST" })
.then(async () => { directoryCache = null; await Promise.all([load(), refreshDirectories()]); })
.catch((error) => { status.textContent = `Showing cached files · ${(error as Error).message}`; })
.finally(() => { mirrorRefresh = null; });
return mirrorRefresh;
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Duplicate refreshDirectories() call on every mirror refresh.

refreshMirror() sets directoryCache = null then runs Promise.all([load(), refreshDirectories()]). Since directoryCache is still null at that point, load()'s own branch (else void refreshDirectories()) fires a second, redundant GET /files/directories concurrently with the explicit one — defeating part of the "coalesced refresh" goal this PR is otherwise achieving.

♻️ Suggested fix: sequence so directoryCache is populated before load() runs
     mirrorRefresh = api(`/api/channels/${channelId}/files/refresh`, { method: "POST" })
-      .then(async () => { directoryCache = null; await Promise.all([load(), refreshDirectories()]); })
+      .then(async () => { directoryCache = null; await refreshDirectories(); await load(); })
       .catch((error) => { status.textContent = `Showing cached files · ${(error as Error).message}`; })
       .finally(() => { mirrorRefresh = null; });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const refreshMirror = (): Promise<void> => {
if (mirrorRefresh) return mirrorRefresh;
status.textContent = "Refreshing from the channel computer…";
mirrorRefresh = api(`/api/channels/${channelId}/files/refresh`, { method: "POST" })
.then(async () => { directoryCache = null; await Promise.all([load(), refreshDirectories()]); })
.catch((error) => { status.textContent = `Showing cached files · ${(error as Error).message}`; })
.finally(() => { mirrorRefresh = null; });
return mirrorRefresh;
};
const refreshMirror = (): Promise<void> => {
if (mirrorRefresh) return mirrorRefresh;
status.textContent = "Refreshing from the channel computer…";
mirrorRefresh = api(`/api/channels/${channelId}/files/refresh`, { method: "POST" })
.then(async () => { directoryCache = null; await refreshDirectories(); await load(); })
.catch((error) => { status.textContent = `Showing cached files · ${(error as Error).message}`; })
.finally(() => { mirrorRefresh = null; });
return mirrorRefresh;
};
🤖 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/channel.ts` around lines 612 - 620, Update refreshMirror so
refreshDirectories() completes and repopulates directoryCache before calling
load(), rather than invoking both concurrently after clearing the cache.
Preserve the existing mirror refresh status, error handling, and refresh
coalescing behavior while ensuring only one directory request is made per
refresh.

Comment on lines +789 to +795
const current = workspaceMirrorRefreshes.get(channelId);
if (current) return current;
const refresh = syncGuestToHost(channelId).finally(() => {
if (workspaceMirrorRefreshes.get(channelId) === refresh) workspaceMirrorRefreshes.delete(channelId);
});
workspaceMirrorRefreshes.set(channelId, refresh);
await refresh;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Await an existing refresh before applying the state gate.

If the first refresh starts while the computer is running, then the computer stops before a second caller arrives, Line [788] returns without awaiting the in-flight promise. prepareChannelWorkspaceArtifact can therefore proceed with a stale mirror. Check workspaceMirrorRefreshes before the running-state guard.

Suggested ordering
 export async function refreshChannelWorkspaceMirror(channelId: number): Promise<void> {
+  const current = workspaceMirrorRefreshes.get(channelId);
+  if (current) return current;
   const computer = channelComputer(channelId);
   if (!computer || !isolatedBackend(computer) || computer.observed_state !== "running") return;
-  const current = workspaceMirrorRefreshes.get(channelId);
-  if (current) return current;
🤖 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/channel-computers.ts` around lines 789 - 795, Update the refresh
flow in prepareChannelWorkspaceArtifact to check workspaceMirrorRefreshes for an
existing promise and await it before applying the computer-running state guard.
Preserve the current deduplication and cleanup behavior for new refreshes, while
ensuring callers arriving after shutdown still wait for any in-flight
syncGuestToHost operation.

Comment thread src/server/db.ts
const configuredBackend = String(process.env.HELM_CHANNEL_COMPUTER_BACKEND || platformBackend);
const backend = ["apple", "lxc", "wsl", "native", "mock"].includes(configuredBackend) ? configuredBackend : platformBackend;
const image = String(process.env.HELM_CHANNEL_MACHINE_IMAGE || "local/1helm-channel-machine:0.0.20");
const image = String(process.env.HELM_CHANNEL_MACHINE_IMAGE || "local/1helm-channel-machine:0.0.21");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Migrate persisted image pins, not only the insert default.

Because image is later supplied through INSERT OR IGNORE at Lines [1021-1027], existing channel_computers rows retain local/1helm-channel-machine:0.0.20. This includes rows whose backend is moved to lxc or wsl, so they may continue provisioning the old image. Add a targeted old-default-to-0.0.21 migration while preserving explicit HELM_CHANNEL_MACHINE_IMAGE overrides, and test an existing workspace.

🤖 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/db.ts` at line 1008, Update the initialization around image and
the channel_computers INSERT OR IGNORE flow to migrate persisted 0.0.20 default
pins to 0.0.21, including rows whose backend is lxc or wsl. Apply this migration
only to the old default value, while preserving any explicit
HELM_CHANNEL_MACHINE_IMAGE override. Add coverage for an existing workspace
containing the old image pin.

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