Skip to content

Release 1.0.4 responsiveness fixes - #85

Merged
gitcommit90 merged 4 commits into
mainfrom
fix/local-responsiveness
Aug 12, 2026
Merged

Release 1.0.4 responsiveness fixes#85
gitcommit90 merged 4 commits into
mainfrom
fix/local-responsiveness

Conversation

@gitcommit90

@gitcommit90 gitcommit90 commented Aug 12, 2026

Copy link
Copy Markdown
Owner

Fixes the local responsiveness regression under disk pressure, bounds Files navigation work, removes repeated inline avatars, and prepares the tested 1.0.4 release.

Summary by CodeRabbit

  • Bug Fixes

    • Improved file loading and navigation in large workspaces.
    • Removed obsolete workspace file records while preserving valid attachments and uploads.
    • Reduced repeated profile-photo data in channel history.
    • Prevented denied service-account commands from repeatedly launching mail processes.
  • Performance

    • Reduced synchronous storage pressure when updating read states.
    • Adjusted background reconciliation timing to minimize startup and ongoing resource usage.
  • Maintenance

    • Updated the application to version 1.0.4.
    • Expanded automated coverage for read-state handling, workspace files, and installer behavior.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

Failed to post review comments.

We encountered an issue with GitHub. Use @coderabbitai full review to retry the review.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 204d24b8-bdf9-4c09-8ecb-8d8fa3e22bca

📥 Commits

Reviewing files that changed from the base of the PR and between 981f433 and b4b49be.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • CHANGELOG.md
  • package.json
  • scripts/run-test-suite.mjs
  • site/public/install-oci-runtime.sh
  • src/server/agents.ts
  • src/server/bots.ts
  • src/server/channel-computers.ts
  • src/server/cowork-contract.ts
  • src/server/database-migrations.ts
  • src/server/db.ts
  • src/server/index.ts
  • src/server/read-state-worker.mjs
  • src/server/read-state.ts
  • src/server/store.ts
  • test/files-latency.mjs
  • test/read-state.mjs
  • test/site.mjs
💤 Files with no reviewable changes (1)
  • src/server/bots.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Use PREVIEW ONLY as the default delivery mode: implement and verify only small, explicitly requested changes, then report them for review.
Unless explicitly authorized, do not bump release versions or notes, create or publish releases/tags/artifacts, deploy the public website, update stable release metadata, or change production data, infrastructure, containers, VMs, or services.
Keep changes focused, avoid broadening scope and unrelated refactors, and pause before changes spread across several domains.
Run the narrowest relevant tests while iterating, then run the full CI contract with npm run ci before merge; never weaken a check to make a change pass.
Modify the module that owns the behavior, extracting a cohesive seam when it keeps ordinary work local.
Use npm run architecture:report to identify legacy hotspots and reduce the relevant budget after making one smaller change.
Every handoff must briefly identify changed files, checks run and their results, known risks, rollback steps, and whether stable or any external system was touched.

Files:

  • scripts/run-test-suite.mjs
  • package.json
  • test/read-state.mjs
  • src/server/database-migrations.ts
  • CHANGELOG.md
  • test/files-latency.mjs
  • src/server/channel-computers.ts
  • src/server/store.ts
  • src/server/read-state-worker.mjs
  • test/site.mjs
  • src/server/read-state.ts
  • site/public/install-oci-runtime.sh
  • src/server/agents.ts
  • src/server/cowork-contract.ts
  • src/server/index.ts
  • src/server/db.ts
📝 Walkthrough

Walkthrough

The release bounds workspace traversal, removes legacy artifact synchronization, cleans unreferenced artifacts, omits avatar data from serialized authors, and moves read-marker persistence to an asynchronous SQLite worker.

Changes

Workspace artifact flow

Layer / File(s) Summary
Workspace artifact flow
src/server/agents.ts, src/server/bots.ts, src/server/cowork-contract.ts, src/server/index.ts, test/files-latency.mjs
Workspace traversal is limited to two levels. Explicit attachments use canonical paths. Legacy synchronization calls are removed. Tests cover cleanup, navigation, and route behavior.

Artifact cleanup and storage configuration

Layer / File(s) Summary
Artifact cleanup and storage configuration
src/server/database-migrations.ts, src/server/db.ts
Unreferenced legacy artifacts are deleted after migrations. SQLite uses WAL with manual checkpointing, storage initialization creates the upload directory, and artifact lookups use a new index.

Asynchronous read-state persistence

Layer / File(s) Summary
Asynchronous read-state persistence
src/server/read-state.ts, src/server/read-state-worker.mjs, src/server/store.ts, src/server/index.ts, test/read-state.mjs, scripts/run-test-suite.mjs
Read markers are queued, coalesced, persisted in transactions, retried after failures, and flushed during shutdown. Integration tests cover worker behavior and SQLite settings.

Payload and release controls

Layer / File(s) Summary
Payload and release controls
src/server/store.ts, src/server/channel-computers.ts, site/public/install-oci-runtime.sh, test/site.mjs, package.json, CHANGELOG.md
Serialized authors omit avatars. Reconciliation defaults are delayed. Sudoers mail notifications are disabled. The package and changelog use version 1.0.4.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ChannelRequest
  participant read-state.ts
  participant read-state-worker.mjs
  participant SQLite
  ChannelRequest->>read-state.ts: queueLastRead(userId, channelId, lastRead)
  read-state.ts->>read-state-worker.mjs: post read update
  read-state-worker.mjs->>SQLite: batch maximum last_read values
  SQLite-->>read-state-worker.mjs: commit transaction
  read-state-worker.mjs-->>read-state.ts: acknowledge flush
Loading

Possibly related PRs

  • gitcommit90/1Helm#30: Both changes update Files latency tests and the test-suite script.
  • gitcommit90/1Helm#46: Both changes remove legacy workspace-artifact synchronization from bot, Cowork, and channel flows.
🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description states the main outcome but omits the required template sections, acceptance ledger, release notes, and verification checklist. Complete the required sections, including change type, release notes, acceptance ledger, verification results, and post-merge items.
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the release and its primary responsiveness fixes.
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.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/local-responsiveness

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

❤️ Share

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

@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: 6

🤖 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 `@scripts/run-test-suite.mjs`:
- Line 22: Rerun the complete npm run ci check with the locked dependencies
installed, including the focused tests and architecture report represented by
the test entries near run-test-suite. Record all command results in the handoff,
including any failures or environment issues.

In `@site/public/install-oci-runtime.sh`:
- Around line 155-156: Update the sudoers template in install-oci-runtime.sh to
include !mail_no_perms alongside the existing mail flags, then update the
related assertion and add coverage for a denied service-user command. Preserve
the CHANGELOG.md entry at lines 24-25 documenting this no-mail guarantee; no
direct change is required there.

In `@src/server/channel-computers.ts`:
- Around line 82-83: Update the timer configuration constants near
RECONCILE_EVERY_MS and INITIAL_RECONCILE_MS to parse finite environment values,
fall back to their existing defaults when invalid, and clamp valid values to
Node’s maximum timer delay of 2147483647 milliseconds while preserving the
existing minimums. Add tests covering invalid and oversized
HELM_FLEET_INTERVAL_MS and HELM_FLEET_INITIAL_MS inputs.

In `@src/server/index.ts`:
- Around line 1324-1326: Update the root-path branch of the workspace files
endpoint to use a bounded direct-directory listing instead of recursively
calling listWorkspaceFiles(channelId). Reuse listWorkspaceDirectory with the
root path, or establish an explicit traversal limit if recursion is required,
and add an endpoint test covering a deep dependency tree to verify the response
remains bounded.

In `@src/server/read-state-worker.mjs`:
- Around line 47-50: Bound read-state flush shutdown across all affected sites:
in src/server/read-state-worker.mjs:47-50, stop or report persistent flush
failures instead of retrying indefinitely; in src/server/read-state.ts:75-82,
enforce a flush deadline, reject pending waiters when it expires, and terminate
the worker; in src/server/index.ts:2275, preserve a bounded shutdown path when
persistence cannot recover; in test/read-state.mjs:20-35, inject a persistent
write failure and assert shutdownReadStateWorker() resolves within the shutdown
deadline.

In `@test/site.mjs`:
- Line 327: Add !mail_no_perms to the installer’s Defaults:%s sudoers policy and
update the ociInstaller assertion to require !mail_badpass, !mail_no_user, and
!mail_no_perms.
🪄 Autofix

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: 204d24b8-bdf9-4c09-8ecb-8d8fa3e22bca

📥 Commits

Reviewing files that changed from the base of the PR and between 981f433 and b4b49be.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (17)
  • CHANGELOG.md
  • package.json
  • scripts/run-test-suite.mjs
  • site/public/install-oci-runtime.sh
  • src/server/agents.ts
  • src/server/bots.ts
  • src/server/channel-computers.ts
  • src/server/cowork-contract.ts
  • src/server/database-migrations.ts
  • src/server/db.ts
  • src/server/index.ts
  • src/server/read-state-worker.mjs
  • src/server/read-state.ts
  • src/server/store.ts
  • test/files-latency.mjs
  • test/read-state.mjs
  • test/site.mjs
💤 Files with no reviewable changes (1)
  • src/server/bots.ts

"test/phase6-modules.mjs",
"test/routing.mjs", "test/routing-disabled-account.mjs", "test/routing-antigravity.mjs", "test/desktop.mjs", "test/update-service.mjs",
"test/channel-computers.mjs", "test/channel-computers-isolated-backends.mjs", "test/event-loop-unblocking.mjs",
"test/channel-computers.mjs", "test/channel-computers-isolated-backends.mjs", "test/event-loop-unblocking.mjs", "test/read-state.mjs",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

node --test test/read-state.mjs
npm run architecture:report
npm run ci

Repository: gitcommit90/1Helm

Length of output: 2644


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- package manifests ---'
git ls-files | grep -E '(^|/)(package\.json|package-lock\.json|npm-shrinkwrap\.json|yarn\.lock|pnpm-lock\.yaml)$' || true

printf '%s\n' '--- package scripts and node dependency declarations ---'
node - <<'JS'
const fs = require('fs');
const p = JSON.parse(fs.readFileSync('package.json', 'utf8'));
console.log(JSON.stringify({
  packageManager: p.packageManager,
  scripts: p.scripts,
  dependencies: p.dependencies,
  devDependencies: p.devDependencies
}, null, 2));
JS

printf '%s\n' '--- TypeScript configuration references ---'
rg -n --glob 'tsconfig*.json' --glob '!node_modules' '"types"|"typeRoots"|"moduleResolution"|"extends"' .

printf '%s\n' '--- relevant tracked files ---'
git ls-files 'test/read-state.mjs' 'scripts/run-test-suite.mjs' 'package.json' 'package-lock.json' 'tsconfig*.json'

Repository: gitcommit90/1Helm

Length of output: 6330


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- lockfile entries for Node types ---'
rg -n -C 2 '"node_modules/@types/node"|"`@types/node`"' package-lock.json

printf '%s\n' '--- installed dependency state ---'
if [ -d node_modules ]; then
  echo 'node_modules: present'
  if [ -d node_modules/@types/node ]; then
    echo '`@types/node`: present'
  else
    echo '`@types/node`: absent'
  fi
else
  echo 'node_modules: absent'
fi

Repository: gitcommit90/1Helm

Length of output: 1527


Run the required checks before merge.

The focused test and architecture report passed. npm run ci requires an environment with the locked dependencies installed; node_modules is absent. Rerun npm run ci and record all results in the handoff.

🤖 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/run-test-suite.mjs` at line 22, Rerun the complete npm run ci check
with the locked dependencies installed, including the focused tests and
architecture report represented by the test entries near run-test-suite. Record
all command results in the handoff, including any failures or environment
issues.

Source: Coding guidelines

Comment on lines +155 to +156
printf 'Defaults:%s !mail_badpass, !mail_no_user\n%s ALL=(root) NOPASSWD: %s *\n' \
"$SERVICE_USER" "$SERVICE_USER" "$HELPER_PATH" >"$TEMP_ROOT/sudoers"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- installer context ---'
sed -n '130,175p' site/public/install-oci-runtime.sh
printf '%s\n' '--- regression test context ---'
sed -n '300,345p' test/site.mjs
printf '%s\n' '--- changelog context ---'
sed -n '15,32p' CHANGELOG.md
printf '%s\n' '--- related sudoers/mail references ---'
rg -n --glob '!node_modules' 'mail_no_(user|perms|badpass)|sudoers|TEMP_ROOT/sudoers|denied' site/public/install-oci-runtime.sh test/site.mjs CHANGELOG.md

Repository: gitcommit90/1Helm

Length of output: 11791


🌐 Web query:

sudoers mail_no_perms mail_no_user Defaults user-specific documentation

💡 Result:

In the sudoers configuration file, mail_no_perms, mail_no_user, and related mail flags are Boolean Defaults options that control whether administrative mail is sent when a user attempts to use sudo under specific conditions [1][2]. 1. mail_no_perms: When set, this flag causes sudo to send mail to the mailto user if the invoking user is permitted to use sudo, but the specific command they are attempting to execute is not listed in their sudoers entry or is explicitly denied [1][2]. This flag is off by default [1][2]. 2. mail_no_user: When set, this flag causes sudo to send mail to the mailto user if the invoking user is not present in the sudoers file at all [1][2]. This flag is on by default [1][2]. Defaults Settings and User-Specific Configuration These flags are defined using the Defaults syntax within the /etc/sudoers file [3]. Defaults entries can be applied globally or restricted to specific users, hosts, or runas users [4][3]. When applying these settings to specific users, the syntax follows the order: Defaults:user_name parameter1, parameter2 Example: To disable mail notifications for a specific user (e.g., 'john'), you could add a user-specific Defaults entry: Defaults:john!mail_no_user,!mail_no_perms Important implementation notes: - Parsing Order: Defaults settings are parsed in the order: global, host, user, runas, and then command-specific defaults [4][3]. If multiple settings of the same type exist, the last matching setting is applied [4][3]. - Overriding: Boolean flags can be enabled by simply listing the name (e.g., mail_no_perms) or disabled by prefixing with an exclamation mark (e.g.,!mail_no_perms) [2]. - Controlling Mail: The destination for these notifications is configured via the mailto Defaults entry, which defaults to root [5][6]. Additionally, the NOMAIL tag can be used on a per-command basis to override these global or user-specific mail settings for specific command executions [5][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re

installer = Path("site/public/install-oci-runtime.sh").read_text()
test = Path("test/site.mjs").read_text()
changelog = Path("CHANGELOG.md").read_text()

rule = re.search(
    r"printf 'Defaults:%s ([^\\n]+)\\n%s ALL=\\(root\\) NOPASSWD: %s \\*\\n'",
    installer,
)
assert rule, "generated sudoers rule not found"
defaults, principal, command = rule.group(1), "%s ALL=(root)", "*"
print("generated Defaults options:", defaults)
print("has !mail_no_user:", "!mail_no_user" in defaults)
print("has !mail_no_perms:", "!mail_no_perms" in defaults)
print("service-user rule grants only helper path with arbitrary arguments:", "NOPASSWD: %s *" in installer)
print("test asserts !mail_no_perms:", "!mail_no_perms" in test)
print("test exercises denied-command path:", bool(re.search(r"denied|not allowed|sudo.*(?:false|cat|sh|id)", test, re.I)))
print("changelog claim present:", "Denied service-user `sudo` calls no longer start mail delivery processes" in changelog)
PY
if command -v man >/dev/null 2>&1; then
  man sudoers 2>/dev/null | col -b 2>/dev/null | grep -A8 -B3 -E 'mail_no_(perms|user)' || true
else
  echo "local sudoers man page unavailable"
fi

Repository: gitcommit90/1Helm

Length of output: 278


🏁 Script executed:

#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
import re

installer = Path("site/public/install-oci-runtime.sh").read_text()
test = Path("test/site.mjs").read_text()
changelog = Path("CHANGELOG.md").read_text()

defaults_line = next(
    line for line in installer.splitlines()
    if "Defaults:%s" in line and "mail_badpass" in line
)
assert "!mail_no_user" in defaults_line
print("defaults line:", defaults_line.strip())
print("has !mail_no_user:", "!mail_no_user" in defaults_line)
print("has !mail_no_perms:", "!mail_no_perms" in defaults_line)

rule_lines = installer.splitlines()
rule_index = next(i for i, line in enumerate(rule_lines) if "ALL=(root)" in line)
rule = "\n".join(rule_lines[rule_index:rule_index + 1])
print("service-user rule:", rule.strip())
print("explicit helper-only command rule:", "NOPASSWD:" in rule and "HELPER_PATH" in rule)

assert "!mail_no_perms" not in test
print("test asserts !mail_no_perms:", "!mail_no_perms" in test)
denied_patterns = re.compile(
    r"(denied|not allowed|permission denied|mail_no_perms|sudo.*(?:false|cat|sh|id))",
    re.I,
)
print("test contains denied-command coverage:", bool(denied_patterns.search(test)))
print(
    "changelog claim present:",
    "Denied service-user `sudo` calls no longer start mail delivery processes" in changelog,
)
PY

Repository: gitcommit90/1Helm

Length of output: 557


Explicitly disable mail for denied service-user commands.

!mail_no_user only applies when the user is absent from sudoers. A host-wide mail_no_perms setting can still start mail for denied commands. Add !mail_no_perms, update the assertion, and cover a denied command. Keep the CHANGELOG.md entry with this guarantee.

📍 Affects 2 files
  • site/public/install-oci-runtime.sh#L155-L156 (this comment)
  • CHANGELOG.md#L24-L25
🤖 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/install-oci-runtime.sh` around lines 155 - 156, Update the
sudoers template in install-oci-runtime.sh to include !mail_no_perms alongside
the existing mail flags, then update the related assertion and add coverage for
a denied service-user command. Preserve the CHANGELOG.md entry at lines 24-25
documenting this no-mail guarantee; no direct change is required there.

Comment on lines +82 to +83
const RECONCILE_EVERY_MS = Math.max(15_000, Number(process.env.HELM_FLEET_INTERVAL_MS || 5 * 60_000));
const INITIAL_RECONCILE_MS = Math.max(25, Number(process.env.HELM_FLEET_INITIAL_MS || 30_000));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- relevant source ---'
sed -n '60,100p' src/server/channel-computers.ts
sed -n '1625,1695p' src/server/channel-computers.ts

printf '%s\n' '--- constant usages ---'
rg -n -C 3 'RECONCILE_EVERY_MS|INITIAL_RECONCILE_MS|HELM_FLEET_INTERVAL_MS|HELM_FLEET_INITIAL_MS|setInterval|setTimeout' src/server/channel-computers.ts

printf '%s\n' '--- related tests ---'
rg -n -C 2 'HELM_FLEET_INTERVAL_MS|HELM_FLEET_INITIAL_MS|reconcil|channel-computers' --glob '*test*' --glob '*spec*' .

Repository: gitcommit90/1Helm

Length of output: 9889


🏁 Script executed:

#!/bin/bash
set -eu

node - <<'JS'
const delays = [NaN, Infinity, -Infinity, 2_147_483_647, 2_147_483_648, 1e15];
for (const delay of delays) {
  const timeout = setTimeout(() => {}, delay);
  const interval = setInterval(() => {}, delay);
  console.log(JSON.stringify({
    delay,
    timeout: { _idleTimeout: timeout._idleTimeout },
    interval: { _idleTimeout: interval._idleTimeout },
  }));
  clearTimeout(timeout);
  clearInterval(interval);
}
JS

Repository: gitcommit90/1Helm

Length of output: 1605


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- reconciliation workload ---'
rg -n -B 12 -A 45 'async function reconcileOne|function reconcileOne|reconcileOne\\(' src/server/channel-computers.ts

printf '%s\n' '--- test setup and environment handling ---'
sed -n '1,180p' test/channel-computers.mjs
rg -n -C 3 'process\\.env|import\\(|channel-computers' test/channel-computers.mjs test/channel-computers-isolated-backends.mjs

Repository: gitcommit90/1Helm

Length of output: 312


🏁 Script executed:

#!/bin/bash
set -eu

printf '%s\n' '--- reconciliation workload ---'
rg -n -F -B 12 -A 45 'reconcileOne' src/server/channel-computers.ts

printf '%s\n' '--- test setup and environment handling ---'
sed -n '1,180p' test/channel-computers.mjs
rg -n -C 3 'process\.env|import\(|channel-computers' test/channel-computers.mjs test/channel-computers-isolated-backends.mjs

Repository: gitcommit90/1Helm

Length of output: 28045


Validate fleet timer overrides before starting reconciliation.

Math.max(...) leaves NaN and values above 2147483647 unchanged. Node converts both to a 1 ms timer. An invalid or oversized HELM_FLEET_INTERVAL_MS can therefore repeatedly start full fleet reconciliation passes. Parse finite values, clamp the upper bound, and use defaults for invalid values for both overrides. Add tests for invalid and oversized inputs.

🤖 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 82 - 83, Update the timer
configuration constants near RECONCILE_EVERY_MS and INITIAL_RECONCILE_MS to
parse finite environment values, fall back to their existing defaults when
invalid, and clamp valid values to Node’s maximum timer delay of 2147483647
milliseconds while preserving the existing minimums. Add tests covering invalid
and oversized HELM_FLEET_INTERVAL_MS and HELM_FLEET_INITIAL_MS inputs.

Comment thread src/server/index.ts
Comment on lines +1324 to +1326
if (!url.searchParams.has("path")) return json(res, 200, { path: "", files: listWorkspaceFiles(channelId) });
const directory = listWorkspaceDirectory(channelId, url.searchParams.get("path") || "");
return json(res, 200, { ...directory, artifacts: q("SELECT * FROM artifacts WHERE channel_id=? ORDER BY modified DESC", channelId) });
return json(res, 200, directory);

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 | 🟠 Major | ⚡ Quick win

Bound the root Files response.

Line 1324 calls listWorkspaceFiles(channelId). That function recursively walks every directory in both trees. A large node_modules tree can still block this request and defeat the responsiveness fix.

Return the direct directory listing for the root path, or add an explicit bounded traversal contract. Add an endpoint test with a deep dependency tree.

Proposed fix
-          if (!url.searchParams.has("path")) return json(res, 200, { path: "", files: listWorkspaceFiles(channelId) });
-          const directory = listWorkspaceDirectory(channelId, url.searchParams.get("path") || "");
+          const directory = listWorkspaceDirectory(channelId, url.searchParams.get("path") || "");
           return json(res, 200, directory);
📝 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 (!url.searchParams.has("path")) return json(res, 200, { path: "", files: listWorkspaceFiles(channelId) });
const directory = listWorkspaceDirectory(channelId, url.searchParams.get("path") || "");
return json(res, 200, { ...directory, artifacts: q("SELECT * FROM artifacts WHERE channel_id=? ORDER BY modified DESC", channelId) });
return json(res, 200, directory);
const directory = listWorkspaceDirectory(channelId, url.searchParams.get("path") || "");
return json(res, 200, directory);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/server/index.ts` around lines 1324 - 1326, Update the root-path branch of
the workspace files endpoint to use a bounded direct-directory listing instead
of recursively calling listWorkspaceFiles(channelId). Reuse
listWorkspaceDirectory with the root path, or establish an explicit traversal
limit if recursion is required, and add an endpoint test covering a deep
dependency tree to verify the response remains bounded.

Comment on lines +47 to +50
} catch (error) {
try { db.exec("ROLLBACK"); } catch {}
parentPort.postMessage({ type: "warning", message: error instanceof Error ? error.message : String(error) });
scheduleFlush(250);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Bound read-state flush failures during shutdown.

A persistent SQLite or filesystem error keeps the worker retrying without acknowledging the flush request. shutdownReadStateWorker() then waits forever, and Line 2275 blocks process shutdown. Read receipts are noncritical, so shutdown must eventually terminate the worker after a bounded flush period.

  • src/server/read-state-worker.mjs#L47-L50: limit retry duration or report a terminal flush failure to pending flush requests.
  • src/server/read-state.ts#L75-L82: apply a flush deadline, reject pending waiters after it, and terminate the worker.
  • src/server/index.ts#L2275-L2275: keep shutdown bounded when read-state persistence cannot recover.
  • test/read-state.mjs#L20-L35: inject a persistent write failure and assert that shutdownReadStateWorker() resolves within the shutdown deadline.
📍 Affects 4 files
  • src/server/read-state-worker.mjs#L47-L50 (this comment)
  • src/server/read-state.ts#L75-L82
  • src/server/index.ts#L2275-L2275
  • test/read-state.mjs#L20-L35
🤖 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/read-state-worker.mjs` around lines 47 - 50, Bound read-state
flush shutdown across all affected sites: in
src/server/read-state-worker.mjs:47-50, stop or report persistent flush failures
instead of retrying indefinitely; in src/server/read-state.ts:75-82, enforce a
flush deadline, reject pending waiters when it expires, and terminate the
worker; in src/server/index.ts:2275, preserve a bounded shutdown path when
persistence cannot recover; in test/read-state.mjs:20-35, inject a persistent
write failure and assert shutdownReadStateWorker() resolves within the shutdown
deadline.

Comment thread test/site.mjs
assert.match(ociManifest, /ONEHELM_OCI_STATE_ROOT="\/var\/lib\/1helm-oci-v1\/runtime\/oci"/, "the clean-slate OCI state has its own fixed data root");
assert.match(ociInstaller, /acl[\s\S]*crun[\s\S]*fuse-overlayfs[\s\S]*podman/, "the installer supplies the complete OCI and direct-access prerequisites");
assert.match(ociInstaller, /visudo -cf/, "the minimal helper-only sudo policy is validated before installation");
assert.match(ociInstaller, /Defaults:%s !mail_badpass, !mail_no_user/, "denied service-user sudo calls never start a mail delivery process inside the hardened service sandbox");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- test/site.mjs context ---'
sed -n '300,340p' test/site.mjs
printf '%s\n' '--- sudo mail flag references ---'
rg -n -C 3 'mail_(badpass|no_perms|no_user)|Defaults:%s' --glob '!node_modules' .
printf '%s\n' '--- candidate installer files ---'
git ls-files | rg -i 'installer|oci|sudo|container|site'

Repository: gitcommit90/1Helm

Length of output: 10692


🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- installer policy context ---'
sed -n '120,175p' site/public/install-oci-runtime.sh
printf '%s\n' '--- helper command and sudo call sites ---'
rg -n -C 4 'sudo|HELPER_PATH|SERVICE_USER|mail_no_perms|mail_no_user|mail_badpass' \
  site/public/install-oci-runtime.sh scripts/1helm-oci-runtime test
printf '%s\n' '--- sudoers documentation available locally ---'
command -v sudo || true
find /usr/share/man /usr/share/doc -type f 2>/dev/null | rg '/sudoers(\.|$)|sudo' | head -20 || true

Repository: gitcommit90/1Helm

Length of output: 17642


🌐 Web query:

sudoers mail_no_perms official sudo documentation

💡 Result:

In the official sudo documentation, mail_no_perms is a configuration option within the sudoers file that controls email notifications regarding permission denials [1][2]. When set to true, sudo will send an email to the user specified in the mailto option if an invoking user who is otherwise allowed to use sudo attempts to run a command that is not listed in their sudoers entry or is explicitly denied [1][2]. This flag is disabled (off) by default [1][2]. Additionally, the behavior of this option can be overridden on a per-command basis using the NOMAIL tag [3][4]. If a NOMAIL tag is applied to a specific command, it will suppress mail notification even if mail_no_perms is enabled [3][1].

Citations:


🏁 Script executed:

#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re

installer = Path("site/public/install-oci-runtime.sh").read_text()
test = Path("test/site.mjs").read_text()

policy = re.search(
    r"printf '([^']*)'\\s*\\n'\s*"
    r'\s*"?(?:\$SERVICE_USER|\$HELPER_PATH)', installer
)
line = next(
    line for line in installer.splitlines()
    if "Defaults:%s" in line and "mail_badpass" in line
)
assert "!mail_badpass" in line
assert "!mail_no_user" in line
assert "!mail_no_perms" not in line

expected = r"/Defaults:%s !mail_badpass, !mail_no_perms, !mail_no_user/"
assert "!mail_no_perms" not in test.splitlines()[326]
print("generated policy template:", line.strip())
print("missing denied-permission flag: !mail_no_perms")
print("proposed test pattern is distinct:", expected)
PY

Repository: gitcommit90/1Helm

Length of output: 407


Disable mail for denied permissions.

Add !mail_no_perms to the installer’s Defaults:%s policy and require it in this assertion. Without the explicit setting, a host-level sudoers default can enable mail for commands that the otherwise permitted service user cannot run.

🤖 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/site.mjs` at line 327, Add !mail_no_perms to the installer’s Defaults:%s
sudoers policy and update the ociInstaller assertion to require !mail_badpass,
!mail_no_user, and !mail_no_perms.

@gitcommit90
gitcommit90 merged commit 079be65 into main Aug 12, 2026
1 check passed
@gitcommit90
gitcommit90 deleted the fix/local-responsiveness branch August 12, 2026 06:02
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