Replace Linux and Windows channel VMs with durable OCI containers - #40
Conversation
Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
Signed-off-by: Joseph Yaksich <gitcommit90@users.noreply.github.com>
📝 WalkthroughWalkthroughThis change replaces LXC/WSL channel isolation with durable OCI containers, runtime-owned channel storage, Podman-based Linux hosting, shared WSL2 provisioning on Windows, OCI installation/update flows, digest-qualified recovery, updated client/server contracts, and corresponding tests and documentation. ChangesOCI runtime migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
|
The exact artifact-producing commit f091325 passed PR CI and CodeQL and was fast-forwarded to main to preserve the already-approved Mac signature/notarization and all cross-platform artifact provenance. Closing this now-fully-contained PR without creating a different squash SHA. |
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (7)
src/server/channel-storage.ts (2)
43-53: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueEach path resolution costs up to three SQLite queries.
channelWorkspacePath/channelFilesPathboth re-enterchannelFilesystemRoot, which querieschannels,channel_computers, andworkspace. These are called per-file in workspace listing/mirroring loops. A small per-channel memo (invalidated on computer row changes) would remove the repeated lookups.🤖 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-storage.ts` around lines 43 - 53, Add a per-channel memo for the resolved root used by channelFilesystemRoot, and have channelWorkspacePath and channelFilesPath reuse that memoized value instead of repeating the SQLite lookups for each file. Invalidate the memo whenever the relevant channel computer/workspace rows change, while preserving current path resolution behavior for both OCI and host backends.
30-36: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWindows state root duplicates the installed manifest value.
The
\var\lib\1helm-oci-v1\runtime\ocisuffix hardcodesONEHELM_OCI_STATE_ROOTfromdeploy/1helm-oci-runtime-v1.conf. If the manifest root is ever bumped (e.g.-v2), Windows silently resolves every channel path under a non-existent directory while Linux keeps working. Derive it from a single shared constant (or read it back from the runtime via the helper) so both sides move together.Also worth noting:
HELM_OCI_HOST_STATE_ROOT/HELM_OCI_STATE_ROOTare accepted unvalidated here, but the helper rejects anything not matching^/.+/runtime/oci$(scripts/1helm-oci-runtime Line 20), so a typo surfaces as an opaque "unsafe runtime state root" failure rather than a config error.🤖 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-storage.ts` around lines 30 - 36, Update ociHostStateRoot and the Windows path construction to derive the runtime state-root suffix from the shared manifest/runtime source instead of hardcoding “1helm-oci-v1”. Preserve the existing environment-variable precedence and non-Windows fallback, and validate HELM_OCI_HOST_STATE_ROOT/HELM_OCI_STATE_ROOT against the same /.../runtime/oci format before returning them so invalid configuration produces the established configuration error.src/server/db.ts (1)
936-942: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUnvalidated backend env value diverges from
channel-storage.ts.Here
HELM_CHANNEL_COMPUTER_BACKENDis validated against the allowed list and falls back toplatformBackend;effectiveBackend()insrc/server/channel-storage.ts(Lines 25-27) accepts the raw value. With a typo, the DB row saysociwhile path routing returns the host tree until the row is read. Share one validated resolver between the two.🤖 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` around lines 936 - 942, The backend selection logic in the database initialization flow must match channel-storage routing for invalid HELM_CHANNEL_COMPUTER_BACKEND values. Extract or reuse a shared validated resolver from the logic around effectiveBackend(), then update the platformBackend/configuredBackend/backend setup in the database flow to use it, preserving the allowed backends and platform-specific fallback.src/server/channel-computers.ts (1)
609-609: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider a dedicated predicate instead of
isolatedBackend(c) && c.backend !== "oci".OCI is an isolated backend but never participates in host↔guest mirroring, so six call sites now repeat the same exclusion. A
mirrorsWorkspace(computer)helper (computer.backend === "apple") would make the invariant explicit and prevent a future backend from silently opting into mirroring.Also applies to: 670-670, 698-698, 739-739, 756-756, 825-825
🤖 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` at line 609, Introduce a dedicated mirrorsWorkspace(computer) predicate that returns true only for the Apple backend, then replace the repeated isolatedBackend(computer) plus OCI exclusion checks at all listed call sites with this predicate. Preserve the existing early-return behavior while centralizing the host↔guest mirroring eligibility rule.test/channel-computers-backend-child.mjs (2)
49-50: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the fixed 2.2 s sleep with a bounded poll.
A single fixed wait for an async in-container write is a flake source under CI load and always burns the full delay. Poll for the file with a deadline instead.
♻️ Proposed refactor
- await new Promise((resolveWait) => setTimeout(resolveWait, 2200)); - assert.equal(readFileSync(join(authoritativeRoot, "workspace", "background.txt"), "utf8"), "background"); + const backgroundPath = join(authoritativeRoot, "workspace", "background.txt"); + const deadline = Date.now() + 20_000; + while (!existsSync(backgroundPath) && Date.now() < deadline) await new Promise((resolveWait) => setTimeout(resolveWait, 100)); + assert.equal(readFileSync(backgroundPath, "utf8"), "background", "background in-container write reaches authoritative storage");🤖 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/channel-computers-backend-child.mjs` around lines 49 - 50, Replace the fixed 2.2-second setTimeout before the background.txt assertion with a bounded polling loop that repeatedly checks for the expected file/content until it matches or a deadline is reached, using a short interval and preserving a clear timeout failure. Keep the existing readFileSync assertion outcome for successful completion.
89-90: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNarrow the call-log negative match to helper operations.
oci-calls.logcontains full argv, including the shell command text sent throughrunChannelCommand. An unanchored/sync/iwill fail on any future test command containingrsync/fsync/"resync" rather than on a real workspace-copy regression. Match the operation token instead, e.g. per-lineJSON.parseand assertargs[0]is never a sync/import/export operation.🤖 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/channel-computers-backend-child.mjs` around lines 89 - 90, Replace the broad raw-text assertion on oci-calls.log with per-line JSON parsing, then inspect each parsed call’s args[0] operation token and assert it is never a workspace sync, import, or export operation. Keep the assertion focused on helper operations so command text containing words like rsync, fsync, or resync does not trigger it.test/fake-oci-runtime.mjs (1)
139-149: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winDerive the backup digest from content so digest verification is actually exercised.
A constant
"a".repeat(64)makesrestore's digest check tautological — a tampered or swapped backup payload still verifies, so the recovery path the child test asserts never proves integrity.createHashis already imported.♻️ Proposed refactor
cpSync(channelDir(name), join(destination, "channel"), { recursive: true, preserveTimestamps: true }); cpSync(configPath(name), join(destination, "config.json")); - const digest = "a".repeat(64); + const hash = createHash("sha256"); + for (const entry of readdirSync(join(destination, "channel"), { recursive: true, withFileTypes: true }).sort((a, b) => (a.name < b.name ? -1 : 1))) { + if (!entry.isFile()) continue; + hash.update(entry.name).update(readFileSync(join(entry.parentPath ?? entry.path, entry.name))); + } + const digest = hash.update(readFileSync(join(destination, "config.json"))).digest("hex"); writeFileSync(join(destination, "digest"), digest);🤖 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/fake-oci-runtime.mjs` around lines 139 - 149, Update the backup branch in the fake OCI runtime to derive the digest written to destination/digest from the backed-up payload using the existing createHash import, rather than a constant value. Hash the same channel/config content that restore verifies and continue returning that computed digest in the JSON response, so restore integrity checks detect tampering or swapped backups.
🤖 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/USER_GUIDE.md`:
- Around line 371-381: Update the host-update preservation guidance in
USER_GUIDE.md to document both Windows durable state roots:
%APPDATA%\1Helm-OCI-v1 and %LOCALAPPDATA%\1Helm-Runtime. Keep the existing macOS
and Linux paths and replacement/deletion guidance unchanged.
In `@scripts/1helm-oci-runtime`:
- Around line 167-184: Update ensure_network so its network exists/create
sequence is serialized with the same installation identity lock used by
ensure_network_identity, or explicitly tolerate a concurrent podman network
create failure when the network now exists. Preserve the ownership-label
validation after either path, ensuring concurrent provisioning for the same
installation does not abort under set -e.
- Around line 368-380: Extend the readiness wait in the container startup loop
around verify_container beyond the current 10-second budget, using a longer
deadline appropriate for systemd initialization and create’s existing 30-minute
allowance. Replace the fixed 0.1-second polling with bounded backoff while
continuing to run the ownership and storage health probe until it succeeds or
the new deadline expires, then retain the existing failure path.
- Around line 568-584: Update the create branch to use an EXIT/ERR cleanup trap,
following the pattern in restore_container, so failures from
create_container—including die during verification—remove the newly created
container and storage when storage_existed is false. Invoke create_container
unconditionally rather than in an if condition, preserving set -e behavior for
ensure_network, ensure_network_identity, and podman create failures. Clear or
disable the trap after successful creation before starting the container.
In `@scripts/install-wsl-runtime.ps1`:
- Around line 20-23: Normalize wsl.exe output by removing embedded null
characters before matching or comparing it. Update Test-PinnedWslRuntime and the
distro-name list handling near the related call site so both version detection
and distro matching use the cleaned output, consistent with the existing
windows-removal.cjs behavior.
In `@site/public/uninstall-host.sh`:
- Line 34: Update the uninstall cleanup command in uninstall-host.sh to also
remove /etc/tmpfiles.d/1helm-oci.conf, preserving the existing forced removal
behavior for all OCI host artifacts.
In `@src/server/agents.ts`:
- Around line 48-63: Update src/server/agents.ts lines 48-63 in
ensureChannelWorkspace to explicitly signal when runtime-owned storage is
expected but the workspace is absent, using a domain error or equivalent flag
before downstream filesystem access. Also update src/server/agents.ts lines
947-950 to guard all three realpathSync calls with existsSync, or consistently
rely on the new ensureChannelWorkspace domain error, so callers receive an
actionable failure instead of a raw filesystem exception.
In `@src/server/channel-computers.ts`:
- Around line 1345-1377: Update runtimeReadiness() to avoid synchronous
spawnSync calls blocking the event loop during the version and readiness probes.
Use asynchronous process execution with awaited results, or reuse a short-lived
cached readiness result refreshed in the background, while preserving the
existing platform-specific parsing and returned readiness fields.
- Around line 558-561: The channel provisioning flow around recordObserved must
stop creating agent-owned runtime directories in the host workspace. Remove or
relocate the mkdirSync call for notes, whiteboards, code, docs, and
presentations so OCI runtime directory creation and ownership remain handled by
the supported container setup, while preserving the subsequent workspace-change
cleanup and status update.
In `@src/server/db.ts`:
- Line 744: Update the channel_computers schema initialization and migration
flow so existing databases replace or widen the old backend CHECK constraint to
include 'oci' before the channel-computer insert loop runs. Preserve existing
rows and ensure both fresh and upgraded databases accept the oci seeding path.
---
Nitpick comments:
In `@src/server/channel-computers.ts`:
- Line 609: Introduce a dedicated mirrorsWorkspace(computer) predicate that
returns true only for the Apple backend, then replace the repeated
isolatedBackend(computer) plus OCI exclusion checks at all listed call sites
with this predicate. Preserve the existing early-return behavior while
centralizing the host↔guest mirroring eligibility rule.
In `@src/server/channel-storage.ts`:
- Around line 43-53: Add a per-channel memo for the resolved root used by
channelFilesystemRoot, and have channelWorkspacePath and channelFilesPath reuse
that memoized value instead of repeating the SQLite lookups for each file.
Invalidate the memo whenever the relevant channel computer/workspace rows
change, while preserving current path resolution behavior for both OCI and host
backends.
- Around line 30-36: Update ociHostStateRoot and the Windows path construction
to derive the runtime state-root suffix from the shared manifest/runtime source
instead of hardcoding “1helm-oci-v1”. Preserve the existing environment-variable
precedence and non-Windows fallback, and validate
HELM_OCI_HOST_STATE_ROOT/HELM_OCI_STATE_ROOT against the same /.../runtime/oci
format before returning them so invalid configuration produces the established
configuration error.
In `@src/server/db.ts`:
- Around line 936-942: The backend selection logic in the database
initialization flow must match channel-storage routing for invalid
HELM_CHANNEL_COMPUTER_BACKEND values. Extract or reuse a shared validated
resolver from the logic around effectiveBackend(), then update the
platformBackend/configuredBackend/backend setup in the database flow to use it,
preserving the allowed backends and platform-specific fallback.
In `@test/channel-computers-backend-child.mjs`:
- Around line 49-50: Replace the fixed 2.2-second setTimeout before the
background.txt assertion with a bounded polling loop that repeatedly checks for
the expected file/content until it matches or a deadline is reached, using a
short interval and preserving a clear timeout failure. Keep the existing
readFileSync assertion outcome for successful completion.
- Around line 89-90: Replace the broad raw-text assertion on oci-calls.log with
per-line JSON parsing, then inspect each parsed call’s args[0] operation token
and assert it is never a workspace sync, import, or export operation. Keep the
assertion focused on helper operations so command text containing words like
rsync, fsync, or resync does not trigger it.
In `@test/fake-oci-runtime.mjs`:
- Around line 139-149: Update the backup branch in the fake OCI runtime to
derive the digest written to destination/digest from the backed-up payload using
the existing createHash import, rather than a constant value. Hash the same
channel/config content that restore verifies and continue returning that
computed digest in the JSON response, so restore integrity checks detect
tampering or swapped backups.
🪄 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: c94c2e46-fe9b-43d9-9cbc-58fba8ed0579
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (57)
CHANGELOG.mdREADME.mdcontainer/Containerfile.ocideploy/1helm-lxc-runtime-v2.confdeploy/1helm-lxc-unprivileged.confdeploy/1helm-oci-runtime-v1.confdesktop/main.cjsdocs/GOVERNANCE.mddocs/USER_GUIDE.mddocs/VISION.mddocs/release-checklist.mddocs/release-lifecycle.mddocs/release-notes-template.mdpackage.jsonpublic/index.htmlscripts/1helm-lxc-netscripts/1helm-lxc-runtimescripts/1helm-oci-runtimescripts/install-wsl-runtime.ps1scripts/package-mac-dmg.cjsscripts/package-windows.cjsscripts/run-test-suite.mjsscripts/windows-removal.cjssite/content.mjssite/manual.htmlsite/public/apply-linux-release.shsite/public/install-linux-units.shsite/public/install-lxc-runtime.shsite/public/install-oci-runtime.shsite/public/install.shsite/public/migrate-linux-host-contract.shsite/public/uninstall-host.shsite/public/update-host.shsrc/client/api.tssrc/client/channel.tssrc/client/onboarding.tssrc/client/settings.tssrc/server/agents.tssrc/server/bots.tssrc/server/channel-computers.tssrc/server/channel-storage.tssrc/server/db.tssrc/server/index.tssrc/server/updates.tstest/channel-computers-backend-child.mjstest/channel-computers-backend-migration.mjstest/channel-computers-isolated-backends.mjstest/channel-computers.mjstest/desktop.mjstest/fake-lxc-runtime.mjstest/fake-oci-runtime.mjstest/fake-wsl.mjstest/native-world.mjstest/production-browser.mjstest/routing-antigravity.mjstest/site.mjstest/update-service.mjs
💤 Files with no reviewable changes (11)
- deploy/1helm-lxc-unprivileged.conf
- scripts/1helm-lxc-net
- deploy/1helm-lxc-runtime-v2.conf
- site/public/migrate-linux-host-contract.sh
- test/fake-lxc-runtime.mjs
- site/public/install-lxc-runtime.sh
- test/fake-wsl.mjs
- scripts/1helm-lxc-runtime
- test/channel-computers-backend-migration.mjs
- test/update-service.mjs
- src/server/updates.ts
| Every host update preserves: | ||
|
|
||
| ```text | ||
| ~/Library/Application Support/1Helm | ||
| ~/Library/Application Support/1Helm-OCI-v1 | ||
| ``` | ||
|
|
||
| On macOS that directory contains databases, credentials, workspaces, resident | ||
| state, and narrow mirrors. Linux preserves the equivalent host state under | ||
| `/var/lib/1helm`. Do not delete either data root during replacement. | ||
| state, and Apple mirrors. Linux preserves the equivalent OCI-generation state | ||
| under `/var/lib/1helm-oci-v1`. The retired data roots remain untouched and are | ||
| not imported by this generation. Do not delete either current data root during | ||
| replacement. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
ast-grep outline desktop/main.cjs --items all
rg -n -C2 'DATA_NAMESPACE|setPath\("userData"|1Helm-Runtime' \
desktop/main.cjs scripts/windows-removal.cjs docs/USER_GUIDE.mdRepository: gitcommit90/1Helm
Length of output: 3024
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Show the guide section with line numbers
sed -n '350,395p' docs/USER_GUIDE.md | cat -n
# Look for any other Windows-specific retention guidance in the guide
rg -n -C2 'Windows|LOCALAPPDATA|1Helm-Runtime|1Helm-OCI-v1|userData|retained' docs/USER_GUIDE.mdRepository: gitcommit90/1Helm
Length of output: 5074
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Inspect the Windows cleanup / data-root handling in the app code
sed -n '1,170p' desktop/main.cjs | cat -n
# Inspect the Windows removal helper
sed -n '1,220p' scripts/windows-removal.cjs | cat -nRepository: gitcommit90/1Helm
Length of output: 13400
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Search for any docs that mention the Windows runtime root by name
rg -n -C2 '1Helm-Runtime|1Helm-OCI-v1|LOCALAPPDATA' .Repository: gitcommit90/1Helm
Length of output: 14683
Document the Windows state roots. Add %APPDATA%\1Helm-OCI-v1 and %LOCALAPPDATA%\1Helm-Runtime here so the recovery guidance covers both durable Windows paths.
🤖 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/USER_GUIDE.md` around lines 371 - 381, Update the host-update
preservation guidance in USER_GUIDE.md to document both Windows durable state
roots: %APPDATA%\1Helm-OCI-v1 and %LOCALAPPDATA%\1Helm-Runtime. Keep the
existing macOS and Linux paths and replacement/deletion guidance unchanged.
| ensure_network() { | ||
| local owner="$1" installation network labels | ||
| installation="$(owner_installation "$owner")" | ||
| network="1helm-$installation" | ||
| if "${PODMAN[@]}" network exists "$network"; then | ||
| labels="$("${PODMAN[@]}" network inspect --format '{{json .Labels}}' "$network")" | ||
| python3 - "$labels" "$installation" <<'PY' | ||
| import json, sys | ||
| labels=json.loads(sys.argv[1] or "{}") | ||
| if labels.get("com.1helm.managed") != "true" or labels.get("com.1helm.installation") != sys.argv[2]: | ||
| raise SystemExit("network ownership labels do not match") | ||
| PY | ||
| else | ||
| "${PODMAN[@]}" network create --disable-dns=false \ | ||
| --label com.1helm.managed=true --label "com.1helm.installation=$installation" "$network" >/dev/null | ||
| fi | ||
| printf '%s' "$network" | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ensure_network check-then-create runs outside the identity lock.
network exists followed by network create is a TOCTOU: two concurrent create invocations for channels of the same installation both miss, and the loser's podman network create fails, aborting provisioning under set -e. The flock in ensure_network_identity (Line 194) is acquired later and does not cover this. Either take the same lock here or treat an "already exists" create failure as success.
🤖 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 `@scripts/1helm-oci-runtime` around lines 167 - 184, Update ensure_network so
its network exists/create sequence is serialized with the same installation
identity lock used by ensure_network_identity, or explicitly tolerate a
concurrent podman network create failure when the network now exists. Preserve
the ownership-label validation after either path, ensuring concurrent
provisioning for the same installation does not abort under set -e.
| for _ in {1..100}; do | ||
| # Podman can briefly fail name-based --user lookup while it reconstructs | ||
| # its ephemeral runroot after a host reboot even though the persisted | ||
| # image passwd database is intact. These identities are pinned by the | ||
| # installed runtime manifest and image contract, so use their numeric form | ||
| # for the readiness probe and every subsequent exec boundary. | ||
| if "${PODMAN[@]}" exec --user 0:0 "$name" /bin/sh -c 'test "$(cat /var/lib/1helm/owner)" = "$1" && test -d /workspace && test -d /home/agent' 1helm-start "$owner" >/dev/null 2>&1; then | ||
| verify_container "$name" "$owner" | ||
| return | ||
| fi | ||
| sleep 0.1 | ||
| done | ||
| die "container did not pass its ownership and storage health check" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
10s readiness budget is tight for a systemd container's first boot.
100 × 0.1s covers systemd reaching a usable state; on a loaded host or first boot after an image build this can expire and die, which then trips the create-cleanup path even though the container was healthy moments later. The caller already allows 30 minutes for create. Consider a longer overall deadline with a backoff instead of a fixed 10s.
🤖 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 `@scripts/1helm-oci-runtime` around lines 368 - 380, Extend the readiness wait
in the container startup loop around verify_container beyond the current
10-second budget, using a longer deadline appropriate for systemd initialization
and create’s existing 30-minute allowance. Replace the fixed 0.1-second polling
with bounded backoff while continuing to run the ownership and storage health
probe until it succeeds or the new deadline expires, then retain the existing
failure path.
| create) | ||
| (($# == 5)) || die "create requires name, owner, CPUs, memory MiB, and image" | ||
| ! container_exists "$1" || die "container already exists" | ||
| storage_existed=0 | ||
| [[ -e "$(channel_root "$1")" ]] && storage_existed=1 | ||
| prepare_storage "$1" "$2" | ||
| if ! create_container "$1" "$2" "$3" "$4" "$5"; then | ||
| container_exists "$1" && "${PODMAN[@]}" rm -f "$1" >/dev/null 2>&1 || true | ||
| if [[ "$storage_existed" -eq 0 ]]; then | ||
| failed_root="$(channel_root "$1")" | ||
| [[ "$failed_root" == "$CHANNELS_ROOT/$1" ]] || die "unsafe failed-create cleanup target" | ||
| rm -rf -- "$failed_root" | ||
| fi | ||
| exit 1 | ||
| fi | ||
| start_container "$1" "$2" | ||
| ;; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
The failed-create cleanup path is unreachable for the most likely failures.
create_container reports errors through die, which exit 1s the whole script (Line 18). So when verify_container at Line 360 rejects the new container, the if ! create_container body never runs: the container and its storage are left behind, and every subsequent create dies at Line 570 with "container already exists" — provisioning is wedged until an operator intervenes manually.
Secondly, calling create_container in a condition context suppresses set -e inside it, so a failure in ensure_network/ensure_network_identity no longer stops the function and podman create proceeds with empty --network/--ip values.
Use an EXIT/ERR trap for cleanup (as restore_container does at Line 497) and call create_container unconditionally.
🤖 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 `@scripts/1helm-oci-runtime` around lines 568 - 584, Update the create branch
to use an EXIT/ERR cleanup trap, following the pattern in restore_container, so
failures from create_container—including die during verification—remove the
newly created container and storage when storage_existed is false. Invoke
create_container unconditionally rather than in an if condition, preserving set
-e behavior for ensure_network, ensure_network_identity, and podman create
failures. Clear or disable the trap after successful creation before starting
the container.
| function Test-PinnedWslRuntime { | ||
| $versionOutput = (& "$env:SystemRoot\System32\wsl.exe" --version 2>&1 | Out-String) | ||
| return $LASTEXITCODE -eq 0 -and $versionOutput -match [regex]::Escape($wslVersion) | ||
| $output = (& $wsl --version 2>&1 | Out-String) | ||
| return $LASTEXITCODE -eq 0 -and $output -match [regex]::Escape($wslVersion) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does wsl.exe --list --quiet still emit UTF-16 output with embedded null characters when captured in PowerShell 5.1 or PowerShell 7 as of 2026?
💡 Result:
Yes, as of July 2026, wsl.exe continues to emit UTF-16LE (Little-Endian) encoded output by default, which manifests as embedded null characters when captured in PowerShell 5.1 or 7 [1][2][3]. While this remains the default behavior for compatibility reasons [4][5], you can resolve this and force wsl.exe to output standard UTF-8 by setting the environment variable WSL_UTF8 to 1 [1][3][5]. To apply this fix in your PowerShell session: $env:WSL_UTF8 = 1 Once this variable is set, wsl.exe commands (including wsl.exe --list --quiet) will produce UTF-8 output, allowing PowerShell to parse the text correctly without needing to strip null characters or adjust console encodings [1][5]. This workaround is widely documented and is the standard approach for ensuring compatibility with PowerShell, scripting environments, and third-party tools [1][4][3].
Citations:
- 1: https://stackoverflow.com/questions/66127118/why-cannot-i-match-for-strings-from-wsl-exe-output
- 2: WSL.exe adds null characters between characters microsoft/WSL#6787
- 3: wsl.exe does not properly parse PowerShell variable for distribution microsoft/WSL#10669
- 4: wsl.exe outputting unicode to stdout microsoft/WSL#4607
- 5: https://stackoverflow.com/questions/67746179/how-do-i-match-on-wsl-output-in-powershell
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- scripts/install-wsl-runtime.ps1 (relevant lines) ---'
nl -ba scripts/install-wsl-runtime.ps1 | sed -n '1,140p'
printf '\n%s\n' '--- scripts/windows-removal.cjs references to wsl output ---'
rg -n --context 2 'replaceAll\("\\0"|wsl --list|wsl --version|wsl.exe|output\(' scripts/windows-removal.cjs scripts/install-wsl-runtime.ps1Repository: gitcommit90/1Helm
Length of output: 252
🏁 Script executed:
set -euo pipefail
printf '%s\n' '--- scripts/install-wsl-runtime.ps1 (lines 1-140) ---'
cat -n scripts/install-wsl-runtime.ps1 | sed -n '1,140p'
printf '\n%s\n' '--- scripts/windows-removal.cjs matches ---'
rg -n -C 2 'replaceAll\("\\0"|wsl --list|wsl --version|wsl\.exe|output\(' scripts/windows-removal.cjs scripts/install-wsl-runtime.ps1Repository: gitcommit90/1Helm
Length of output: 13159
Strip \0 from wsl.exe output before comparing. scripts/install-wsl-runtime.ps1:20-23,97 — wsl.exe can still return UTF-16LE text with embedded nulls in PowerShell, so both the version check and the distro-name list can miss valid matches and trigger unnecessary reinstall/import paths. scripts/windows-removal.cjs already removes \0 for the same reason.
🤖 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 `@scripts/install-wsl-runtime.ps1` around lines 20 - 23, Normalize wsl.exe
output by removing embedded null characters before matching or comparing it.
Update Test-PinnedWslRuntime and the distro-name list handling near the related
call site so both version detection and distro matching use the cleaned output,
consistent with the existing windows-removal.cjs behavior.
| rm -f -- /etc/sudoers.d/1helm-lxc-runtime /etc/1helm/lxc-runtime-v2.conf /usr/libexec/1helm-lxc-runtime /usr/libexec/1helm-lxc-net | ||
| systemctl disable --now 1helm-update.path 2>/dev/null || true | ||
| rm -f -- /etc/systemd/system/1helm.service /etc/systemd/system/1helm-update.service /etc/systemd/system/1helm-update.path | ||
| rm -f -- /etc/sudoers.d/1helm-oci-runtime /etc/1helm/oci-runtime-v1.conf /usr/libexec/1helm-oci-runtime /usr/lib/1helm-oci/Containerfile.oci |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Remove the OCI tmpfiles policy too.
/etc/tmpfiles.d/1helm-oci.conf is part of the OCI host contract but is left behind. Remove it on uninstall so systemd-tmpfiles cannot recreate stale runtime paths after the host is removed.
Proposed fix
-rm -f -- /etc/sudoers.d/1helm-oci-runtime /etc/1helm/oci-runtime-v1.conf /usr/libexec/1helm-oci-runtime /usr/lib/1helm-oci/Containerfile.oci
+rm -f -- /etc/sudoers.d/1helm-oci-runtime /etc/1helm/oci-runtime-v1.conf /etc/tmpfiles.d/1helm-oci.conf /usr/libexec/1helm-oci-runtime /usr/lib/1helm-oci/Containerfile.oci📝 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.
| rm -f -- /etc/sudoers.d/1helm-oci-runtime /etc/1helm/oci-runtime-v1.conf /usr/libexec/1helm-oci-runtime /usr/lib/1helm-oci/Containerfile.oci | |
| rm -f -- /etc/sudoers.d/1helm-oci-runtime /etc/1helm/oci-runtime-v1.conf /etc/tmpfiles.d/1helm-oci.conf /usr/libexec/1helm-oci-runtime /usr/lib/1helm-oci/Containerfile.oci |
🤖 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 `@site/public/uninstall-host.sh` at line 34, Update the uninstall cleanup
command in uninstall-host.sh to also remove /etc/tmpfiles.d/1helm-oci.conf,
preserving the existing forced removal behavior for all OCI host artifacts.
| export function ensureChannelWorkspace(channelId: number): string { | ||
| const channel = q1("SELECT id,name,personal_main_owner_id FROM channels WHERE id=? AND status<>'deleted'", channelId); | ||
| if (!channel) throw new Error("Channel workspace not found."); | ||
| const root = channelRoot(channelId); | ||
| for (const dir of WORLD_DIRS) mkdirSync(join(root, dir), { recursive: true }); | ||
| for (const dir of COWORK_DIRS) { | ||
| const path = join(root, "workspace", dir); | ||
| if (!existsSync(path)) { | ||
| mkdirSync(path, { recursive: true }); | ||
| markWorkspaceDirty(channelId, `workspace/${dir}`, "upsert"); | ||
| for (const dir of WORLD_DIRS.filter((entry) => entry !== "workspace" && entry !== "files")) mkdirSync(join(root, dir), { recursive: true }); | ||
| if (!channelUsesRuntimeStorage(channelId) || existsSync(channelWorkspace(channelId))) { | ||
| mkdirSync(channelWorkspace(channelId), { recursive: true }); | ||
| mkdirSync(channelFiles(channelId), { recursive: true }); | ||
| for (const dir of COWORK_DIRS) { | ||
| const path = join(channelWorkspace(channelId), dir); | ||
| if (!existsSync(path)) { | ||
| mkdirSync(path, { recursive: true }); | ||
| markWorkspaceDirty(channelId, `workspace/${dir}`, "upsert"); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ensureChannelWorkspace no longer guarantees the workspace/files roots for OCI channels, but callers still assume it does. Runtime-owned storage means those directories appear only after the container is provisioned, so every caller that stats or resolves them can now throw a raw filesystem error instead of a domain error.
src/server/agents.ts#L48-L63: signal the unready state explicitly (throw a domain error, or return a flag) when runtime storage is expected but absent, so downstream surfaces render an actionable message.src/server/agents.ts#L947-L950: guard the threerealpathSynccalls withexistsSync(as line 923 already does) or rely on the new domain error fromensureChannelWorkspace.
📍 Affects 1 file
src/server/agents.ts#L48-L63(this comment)src/server/agents.ts#L947-L950
🤖 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/agents.ts` around lines 48 - 63, Update src/server/agents.ts lines
48-63 in ensureChannelWorkspace to explicitly signal when runtime-owned storage
is expected but the workspace is absent, using a domain error or equivalent flag
before downstream filesystem access. Also update src/server/agents.ts lines
947-950 to guard all three realpathSync calls with existsSync, or consistently
rely on the new ensureChannelWorkspace domain error, so callers receive an
actionable failure instead of a raw filesystem exception.
| recordObserved(computer, inspection); | ||
| run("UPDATE channel_computers SET provision_status='ready',desired_state='auto',last_update=?,last_update_attempt=?,last_error='',updated=? WHERE channel_id=?", now(), now(), now(), computer.channel_id); | ||
| for (const directory of ["notes", "whiteboards", "code", "docs", "presentations"]) mkdirSync(join(hostWorkspace(computer.channel_id), directory), { recursive: true }); | ||
| run("DELETE FROM channel_workspace_changes WHERE channel_id=?", computer.channel_id); | ||
| run("UPDATE channel_computers SET provision_status='ready',desired_state='auto',synced_host_revision=host_revision,last_update=?,last_update_attempt=?,last_error='',updated=? WHERE channel_id=?", now(), now(), now(), computer.channel_id); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'notes|whiteboards|presentations|chown' scripts/1helm-oci-runtimeRepository: gitcommit90/1Helm
Length of output: 423
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== scripts/1helm-oci-runtime around ownership setup ==\n'
sed -n '300,560p' scripts/1helm-oci-runtime
printf '\n== src/server/channel-computers.ts around the provision path ==\n'
sed -n '520,575p' src/server/channel-computers.tsRepository: gitcommit90/1Helm
Length of output: 17093
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "hostWorkspace|ociHostStateRoot|mkdirSync\\(" src scripts -g '!**/node_modules/**'Repository: gitcommit90/1Helm
Length of output: 4826
mkdirSync here is still creating host-owned runtime directories in the OCI storage tree. scripts/1helm-oci-runtime only fixes ownership for workspace/, files/, and home/; it does not apply any agent ownership to the notes/, whiteboards/, code/, docs/, or presentations directories created on the host here, so a permission mismatch or post-recordObserved failure still looks possible.
🤖 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 558 - 561, The channel
provisioning flow around recordObserved must stop creating agent-owned runtime
directories in the host workspace. Remove or relocate the mkdirSync call for
notes, whiteboards, code, docs, and presentations so OCI runtime directory
creation and ownership remain handled by the supported container setup, while
preserving the subsequent workspace-change cleanup and status update.
| if (backend === "oci") { | ||
| let helper = "", version = "", system: unknown = null, error = ""; | ||
| try { | ||
| helper = resolveLxcHelper(); | ||
| const prefix = process.env.HELM_LXC_HELPER_USE_SUDO === "0" || process.getuid?.() === 0 ? [] : ["-n", helper]; | ||
| const executable = prefix.length ? "sudo" : helper; | ||
| const versionResult = spawnSync(executable, [...prefix, "version"], { encoding: "utf8", timeout: 10_000 }); | ||
| version = versionResult.status === 0 ? String(versionResult.stdout || "").trim() : ""; | ||
| const readyResult = spawnSync(executable, [...prefix, "ready"], { encoding: "utf8", timeout: 15_000 }); | ||
| if (readyResult.status === 0) { | ||
| try { system = JSON.parse(String(readyResult.stdout || "")); } catch { system = String(readyResult.stdout || "").trim(); } | ||
| } else error = String(readyResult.stderr || readyResult.stdout || "LXC runtime readiness check failed.").trim(); | ||
| if (platform() === "win32") { | ||
| helper = `${installationScopedRuntimeName()}:/usr/libexec/1helm-oci-runtime`; | ||
| const invocation = ociInvocation(["version"]); | ||
| const versionResult = spawnSync(invocation.command, invocation.args, { encoding: "buffer", timeout: 15_000, env: invocation.env }); | ||
| version = versionResult.status === 0 ? windowsLines(versionResult.stdout as Buffer).join("").trim() : ""; | ||
| const readyInvocation = ociInvocation(["ready"]); | ||
| const readyResult = spawnSync(readyInvocation.command, readyInvocation.args, { encoding: "buffer", timeout: 30_000, env: readyInvocation.env }); | ||
| if (readyResult.status === 0) { | ||
| try { system = JSON.parse(Buffer.from(readyResult.stdout as Buffer).toString("utf8")); } catch { system = windowsLines(readyResult.stdout as Buffer); } | ||
| } else error = windowsLines(Buffer.concat([readyResult.stderr as Buffer || Buffer.alloc(0), readyResult.stdout as Buffer || Buffer.alloc(0)])).join(" ") || "OCI runtime readiness check failed."; | ||
| } else { | ||
| const versionInvocation = ociInvocation(["version"]); | ||
| helper = versionInvocation.command; | ||
| const versionResult = spawnSync(versionInvocation.command, versionInvocation.args, { encoding: "utf8", timeout: 15_000, env: versionInvocation.env }); | ||
| version = versionResult.status === 0 ? String(versionResult.stdout || "").trim() : ""; | ||
| const readyInvocation = ociInvocation(["ready"]); | ||
| const readyResult = spawnSync(readyInvocation.command, readyInvocation.args, { encoding: "utf8", timeout: 30_000, env: readyInvocation.env }); | ||
| if (readyResult.status === 0) { | ||
| try { system = JSON.parse(String(readyResult.stdout || "")); } catch { system = String(readyResult.stdout || "").trim(); } | ||
| } else error = String(readyResult.stderr || readyResult.stdout || "OCI runtime readiness check failed.").trim(); | ||
| } | ||
| } catch (failure) { error = (failure as Error).message; } | ||
| const supported = linux && supportedArchitecture; | ||
| const supported = (linux || windows) && supportedArchitecture; | ||
| return { | ||
| backend, supported, ready: Boolean(supported && helper && version === LXC_RUNTIME_VERSION && system && !error), | ||
| backend, supported, ready: Boolean(supported && helper && version === OCI_RUNTIME_VERSION && system && !error), | ||
| platform: platform(), architecture: process.arch, cli: helper || null, version: version || null, system, | ||
| runtime_version: LXC_RUNTIME_VERSION, status: error ? "error" : system ? "running" : "missing", error: error || null, | ||
| }; | ||
| } | ||
| if (backend === "wsl") { | ||
| let cli = "", version: unknown = null, system: unknown = null, error = ""; | ||
| try { | ||
| cli = resolveWslCli(); | ||
| const versionResult = spawnSync(cli, ["--version"], { encoding: "buffer", timeout: 10_000 }); | ||
| if (versionResult.status === 0) version = windowsLines(versionResult.stdout as Buffer); | ||
| const statusResult = spawnSync(cli, ["--status"], { encoding: "buffer", timeout: 15_000 }); | ||
| if (statusResult.status === 0) system = windowsLines(statusResult.stdout as Buffer); | ||
| else error = windowsLines(Buffer.concat([statusResult.stderr as Buffer || Buffer.alloc(0), statusResult.stdout as Buffer || Buffer.alloc(0)])).join(" ") || "WSL 2 readiness check failed."; | ||
| } catch (failure) { error = (failure as Error).message; } | ||
| const supported = windows && supportedArchitecture; | ||
| const artifact = supportedArchitecture ? WSL_ROOTFS_ARTIFACTS[arm64 ? "arm64" : "amd64"] : null; | ||
| return { | ||
| backend, supported, ready: Boolean(supported && cli && system && !error), | ||
| platform: platform(), architecture: process.arch, cli: cli || null, version, system, | ||
| runtime_version: WSL_RUNTIME_VERSION, rootfs_release: WSL_ROOTFS_RELEASE, | ||
| rootfs_name: artifact?.name || null, rootfs_sha256: artifact?.sha256 || null, | ||
| runtime_version: OCI_RUNTIME_VERSION, shared_runtime: windows ? installationScopedRuntimeName() : null, | ||
| storage_authority: windows ? `\\\\wsl.localhost\\${installationScopedRuntimeName()}\\var\\lib\\1helm-oci-v1\\runtime\\oci` : ociHostStateRoot(), | ||
| status: error ? "error" : system ? "running" : "missing", error: error || null, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
runtimeReadiness() blocks the event loop for up to ~45 s per call.
Both branches use spawnSync with 15 s + 30 s timeouts, and on Windows each call crosses into WSL (cold-start of the distribution can consume most of that budget). This function is called synchronously from several request paths — /api/computers, /api/channel-computers/runtime, /api/channels/:id/computer, /api/setup/complete, and twice more in runtime/install / runtime/start — so a stalled or missing runtime freezes the whole server, including unrelated requests and WebSocket traffic.
Consider making the probe async (spawn + await) or caching the result for a few seconds and refreshing it in the background.
🤖 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 1345 - 1377, Update
runtimeReadiness() to avoid synchronous spawnSync calls blocking the event loop
during the version and readiness probes. Use asynchronous process execution with
awaited results, or reuse a short-lived cached readiness result refreshed in the
background, while preserving the existing platform-specific parsing and returned
readiness fields.
| CREATE TABLE IF NOT EXISTS channel_computers ( | ||
| channel_id INTEGER PRIMARY KEY REFERENCES channels(id) ON DELETE CASCADE, | ||
| backend TEXT NOT NULL CHECK (backend IN ('apple','lxc','wsl','native','mock')), | ||
| backend TEXT NOT NULL CHECK (backend IN ('apple','oci','native','mock')), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm nothing else rebuilds channel_computers or rewrites legacy backends.
rg -nP --type=ts -C3 "channel_computers" src/server/db.ts | rg -nP -C3 "DROP|RENAME|backend|lxc|wsl"
rg -nP -C3 "'lxc'|\"lxc\"|'wsl'|\"wsl\"" src test scriptsRepository: gitcommit90/1Helm
Length of output: 1058
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant sections in src/server/db.ts.
sed -n '700,980p' src/server/db.ts | cat -n
# Find any other channel_computers rebuild/drop logic and legacy backend rewrites.
rg -n "DROP TABLE|ALTER TABLE|CREATE TABLE IF NOT EXISTS channel_computers|backend IN \(|lxc|wsl|oci" src/server/db.ts src test scriptsRepository: gitcommit90/1Helm
Length of output: 48789
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the backend-reading path and any fallback behavior.
sed -n '1,140p' src/server/channel-storage.ts | cat -n
sed -n '1,220p' src/server/channel-computers.ts | cat -n
# Look for any schema migration helpers applied to channel_computers elsewhere.
rg -n "channel_computers|effectiveBackend|backend\s*===" src/server src/client test scriptsRepository: gitcommit90/1Helm
Length of output: 36312
Migrate channel_computers on upgrade
CREATE TABLE IF NOT EXISTS leaves the old CHECK (backend IN ('apple','lxc','wsl','native','mock')) in place on upgraded databases, so the new backend='oci' seeding path can’t populate the table. Rebuild or widen this schema before the insert loop runs, or upgraded channels won’t get control-plane rows.
🤖 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 744, Update the channel_computers schema
initialization and migration flow so existing databases replace or widen the old
backend CHECK constraint to include 'oci' before the channel-computer insert
loop runs. Preserve existing rows and ensure both fresh and upgraded databases
accept the oci seeding path.
Outcome
Ships the deliberate clean-start 0.0.29 desktop architecture: native Podman OCI channel computers on Linux, one managed WSL 2 OCI runtime on Windows, and the existing isolated Apple container backend on macOS. Includes the ReRouted 0.5.10 Antigravity CRLF streaming fix.
Acceptance ledger
f09132556c56df59c4c4c210da8d0d8fb65bc4df, Developer ID signed, Apple notarized and stapled, accepted by Gatekeeper, installed onmacstudiojoseph, and approved by the owner./workspace, Files, persistence, resize, stop/start network identity, digest-qualified backup rejection/restore, repeat install, and host reboot preservation.NotSigned; no self-signing is used.Verification
npm run typechecknpm run buildnpm test— 125 integration assertions plus 111 Node tests passed; 2 browser-dependent tests skipped because Chrome is absentnpm run test:onboarding-browser— 20/20 passedgit diff --checkSummary by CodeRabbit
New Features
Bug Fixes
Documentation