Add production bot ops launcher

[phroi]

Why

Production bot operation needs a small deployment surface for durable file logs, incident collection, and systemd wiring without changing app runtime or SDK behavior.

Changes

• Add a production bot launcher that routes stdout, stderr, and launch metadata into bot-only file logs with path and symlink safety checks.
• Add an incident bundle collector for bot logs, launcher records, version metadata, and optional systemd status/journal/unit captures.
• Harden systemd install/update scripts around launcher wiring, file log roots, encrypted credentials, restart prevention for exit code 2, and core-dump disabling.
• Document launcher, logging, incident, logrotate, credential, and systemd operations in the bot README.

[gemini-code-assist]

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production process launcher (ickb-bot-launcher.mjs) and an incident collection tool (ickb-bot-collect-incident.mjs) to improve logging, crash diagnostics, and core-dump hardening for the bot services. It also updates the systemd installation and update scripts to wire up the launcher and enforce validation checks. The feedback suggests correcting potential multi-byte UTF-8 decoding errors on chunk boundaries in the incident collector using a streaming TextDecoder, improving cross-platform path resolution in the launcher's entry point check, and making systemd unit parsing in the update script more resilient to whitespace formatting around the = operator.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production process launcher (ickb-bot-launcher.mjs) and an incident log collector (ickb-bot-collect-incident.mjs) to manage file logging, launcher metadata, and diagnostic bundling for the bot service. It also updates the systemd installation and update scripts to wire services through the new launcher. The review comments point out critical improvements: handling potential asynchronous spawn errors in the launcher, resolving configured paths to absolute paths early to prevent false-positive symlink errors on systems with benign system-level symlinks, fixing a potential line-skipping bug in the Bash update script when reading files without a trailing newline, and wrapping Git metadata resolution in a try-catch block with explicit error checks to prevent unexpected crashes during incident collection.

I am having trouble creating individual review comments. Click here to see my feedback.

scripts/ickb-bot-launcher.mjs (177-184)

If the child process fails to spawn asynchronously (for example, if the command executable is not found or lacks execution permissions), the waitForChild helper catches the "error" event and returns it in the error property of childResult. However, this error is currently swallowed silently, and the launcher exits with status 1 without printing any diagnostic message to stderr or logging it.

To improve debuggability, check if childResult.error is defined and print/log it before exiting.

    if (copyResult !== undefined) {
      stderr.write(`ickb-bot-launcher: ${publicErrorMessage(copyResult)}\n`);
      return { status: 1 };
    }
    if (childResult.error !== undefined) {
      stderr.write(`ickb-bot-launcher: Failed to spawn child process: ${publicErrorMessage(childResult.error)}\n`);
      return { status: 1 };
    }
    if (childResult.signal !== null) {
      return { signal: childResult.signal };
    }
    return { status: childResult.status ?? 1 };

scripts/ickb-bot-launcher.mjs (251-256)

The strict check real !== path in proveResolvedPath (and the corresponding component-by-component symlink checks in the collector) will cause the launcher to fail to start if any parent directory in the path ancestry is a benign system-level symlink (for example, if /var/log is symlinked to another partition, or /home is symlinked to /usr/home). To prevent symlink-chasing vulnerabilities without sacrificing operational flexibility, resolve the configured paths to their canonical, absolute paths using realpath at the very beginning of path resolution, and then perform all containment and write operations using those canonical paths.

References

1. On systems like macOS where system temporary directories (e.g., '/var') are symlinked to '/private/var', resolve root directories using 'realpath' before performing path validations, ignored-path checks, or parent directory creation to prevent false-positive symlink errors.

scripts/ickb-bot-systemd-update.sh (106-117)

In Bash, the read builtin returns a non-zero exit status when it encounters the end-of-file (EOF) without a trailing newline. If a systemd service unit file is edited or saved without a trailing newline, the last line of the file will be silently skipped by this loop. To ensure all lines are processed correctly regardless of the trailing newline, use the || [[ -n ${line} ]] pattern.

  while IFS= read -r line || [[ -n ${line} ]]; do
    line=${line%$'\r'}
    [[ ${line} =~ ^[[:space:]]*($|#|\;) ]] && continue
    if [[ ${line} =~ ^[[:space:]]*\[(.*)\][[:space:]]*$ ]]; then
      [[ ${BASH_REMATCH[1]} == Service ]] && in_service=1 || in_service=0
      continue
    fi
    [[ ${in_service} -eq 1 ]] || continue
    if value=$(unit_directive_value "${line}" "${key}") && [[ ${value} == "${expected}" ]]; then
      return 0
    fi
  done <<<"${unit_text}"

scripts/ickb-bot-collect-incident.mjs (634-642)

If spawnSync throws an unexpected error, it will crash the entire incident collection process. Wrapping the spawnSync call in a try...catch block ensures that a Git metadata resolution failure does not block the collection of critical log files. Additionally, when executing external system commands via spawnSync, explicitly check for execution errors (such as result.error when the executable is missing) and non-zero exit statuses rather than assuming a null status indicates success.

function readGitCommit(root, dependencies) {
  try {
    const spawn = dependencies.spawnSync ?? spawnSync;
    const result = spawn("git", ["-C", root, "rev-parse", "HEAD"], { encoding: "utf8", timeout: 5_000 });
    if (result.error || result.status !== 0) {
      return null;
    }
    const commit = result.stdout.trim();
    return commit === "" ? null : commit;
  } catch {
    return null;
  }
}

References

1. When executing external system commands (such as 'git') via 'spawnSync', explicitly check for execution errors (e.g., 'result.error' when the executable is missing) and non-zero/unexpected exit statuses rather than assuming a null status indicates a specific logical outcome.

[phroi]

Addressed the second-pass review items in the latest push:

• Launcher spawn failures now report a diagnostic instead of silently returning status 1.
• Incident collection keeps git commit provenance best-effort, returning null if git cannot run.
• Systemd unit parsing now handles files without a trailing newline.
• The symlink-root suggestion is intentionally unchanged: these production log paths refuse symlinked roots, parents, and files rather than canonicalizing to a different write target.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production process launcher (scripts/ickb-bot-launcher.mjs) and an incident collector (scripts/ickb-bot-collect-incident.mjs) along with corresponding tests. It also updates the systemd installation and update scripts to wire the bot execution through the new launcher, enforce core-dump hardening, and validate unit configurations. The README has been updated to document the new logging layout, incident bundling, and log rotation. The review feedback highlights two important issues in the launcher script: a critical deadlock risk in copyBytes if a write error occurs while the stream is paused, and a potential file descriptor leak in LogSink.close() if a pending write promise is rejected.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production launcher script (ickb-bot-launcher.mjs) to manage process plumbing and file logging for the bot, alongside an incident collector script (ickb-bot-collect-incident.mjs) to bundle logs and systemd diagnostics. It also updates the systemd installation and update scripts to integrate the launcher, enforce core-dump hardening, and validate unit configurations. Feedback on these changes highlights a potential issue in copyBytes where stream write errors are swallowed rather than propagated, and notes that the exact string matching in unit_has_directive is fragile and may fail on customized systemd units.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production process launcher (ickb-bot-launcher.mjs) and an incident collector (ickb-bot-collect-incident.mjs) for the iCKB bot. It updates the systemd installation and update scripts to wire the bot through the launcher, configure secure log directories, enforce core-dump hardening, and support customizable log roots. Additionally, the credential helper is updated to support optional RPC URLs and retry budgets, accompanied by comprehensive documentation in the README. The review comments suggest minor improvements to robustly validate launcher arguments against flag swallowing and to add backticks to the disallowed character set in the systemd path validator for defense-in-depth.

[phroi]

/gemini review

[gemini-code-assist]

Code Review

This pull request introduces a production launcher (ickb-bot-launcher.mjs) and an incident collector (ickb-bot-collect-incident.mjs) to improve bot logging, process supervision, and crash diagnostics. It updates the systemd installation and update scripts to wire the bot through the new launcher, enforce core-dump hardening, and validate unit configurations before applying updates. Additionally, it updates the credential helper to support optional RPC URLs and retryable attempt budgets, adds comprehensive unit tests for all new scripts, and updates the README documentation. I have no feedback to provide as there are no review comments.

[phroi]

LGTM

Phroi %196