Fix install-apt test return after rebase

Polish PR review CLI and harden install-apt (v1.2.1)
- Accept approve/approved review event aliases; print review html_url - Harden pr show against missing user/head/base; clarify body-file wins - Add tests for whitespace-only comments, approved alias, review URL - Timeout the CLI-boundary HTTP fixture; clean up install-apt test trees - Clearer refuse-to-overwrite message when nodesource.list already exists - Merge Node 22 NodeSource bootstrap (from fix/apt-nodejs-bootstrap)
2026-07-23 00:08:06 +00:00 · 2026-07-23 00:06:15 +00:00 · 2026-07-22 23:19:04 +00:00 · 2026-07-22 23:17:20 +00:00 · 2026-07-22 22:01:51 +00:00 · 2026-07-22 21:56:26 +00:00
7 changed files with 355 additions and 42 deletions
--- a/README.md
+++ b/README.md
@ -19,7 +19,7 @@ The package is published to the Debian registry of the forge itself. One-time se
 curl -fsSL https://forgejo.heavyduty.builders/heavy-duty/stoke/raw/branch/main/scripts/install-apt.sh | bash
 ```
-or manually:
+or manually. First add the forge's registry as an apt source:
 ```bash
 sudo install -d /etc/apt/keyrings
@ -27,10 +27,24 @@ curl -fsSL https://forgejo.heavyduty.builders/api/packages/heavy-duty/debian/rep
  | sudo tee /etc/apt/keyrings/forgejo-heavy-duty.asc >/dev/null
 echo "deb [signed-by=/etc/apt/keyrings/forgejo-heavy-duty.asc] https://forgejo.heavyduty.builders/api/packages/heavy-duty/debian stable main" \
  | sudo tee /etc/apt/sources.list.d/forgejo-heavy-duty.list
 ```
 The package depends on `nodejs (>= 22.12)`, which the distro archives of Debian 13 (Node 20) and Ubuntu 24.04 (Node 18) cannot satisfy — on those distros, also add a Node 22 source such as [NodeSource](https://deb.nodesource.com):
 ```bash
 curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key \
  | sudo tee /etc/apt/keyrings/nodesource.asc >/dev/null
 echo "deb [signed-by=/etc/apt/keyrings/nodesource.asc] https://deb.nodesource.com/node_22.x nodistro main" \
  | sudo tee /etc/apt/sources.list.d/nodesource.list
 ```
 Then install:
 ```bash
 sudo apt-get update && sudo apt-get install stoke
 ```
-Upgrades then arrive through regular `apt-get upgrade`.
+Upgrades then arrive through regular `apt-get upgrade`. `install-apt.sh` performs all of the above, adding the NodeSource repository only when no already-configured apt source offers a new-enough nodejs.
 Note: apt releases that verify OpenPGP with `sqv` (Debian 13+, apt >= 2.9) currently reject the signature Forgejo generates for its Debian registry (an upstream signing bug). `install-apt.sh` detects this and falls back to a `[trusted=yes]` source — integrity then relies on HTTPS to the forge. The script prefers the signed source, so setups heal automatically once the forge is fixed.
@ -457,15 +471,15 @@ Calls `GET /api/v1/repos/{owner}/{repo}/pulls/{number}`.
 ### `stoke pr comment`
-Add a comment to a pull request.
+Add a comment to a pull request. Body is required (whitespace-only is rejected). When both `-b` and `--body-file` are set, **`--body-file` wins**.
 ```text
 Options:
  -o, --owner <owner>    repository owner (required)
  -r, --repo <repo>      repository name (required)
  -n, --number <number>  pull request number (required)
-  -b, --body <body>      comment body (markdown)
+  -b, --body <body>      comment body (markdown; required unless --body-file)
-  --body-file <path>     read the comment body from a file
+  --body-file <path>     read the comment body from a file (wins over -b)
 ```
 ```bash
@ -476,23 +490,24 @@ Calls `POST /api/v1/repos/{owner}/{repo}/issues/{number}/comments`.
 ### `stoke pr review`
-Submit a review on a pull request.
+Submit a review on a pull request. `approve` / `approved` may omit a body; `request-changes` and `comment` require a non-empty body (raw body is preserved — only emptiness is checked with `trim()`). When both `-b` and `--body-file` are set, **`--body-file` wins**.
 ```text
 Options:
  -o, --owner <owner>      repository owner (required)
  -r, --repo <repo>        repository name (required)
  -n, --number <number>    pull request number (required)
-  --event <event>          review event: approve, request-changes|request_changes, comment (required)
+  --event <event>          approve|approved, request-changes|request_changes, comment (required)
-  -b, --body <body>        review body (markdown)
+  -b, --body <body>        review body (markdown; required for request-changes and comment)
-  --body-file <path>       read the review body from a file
+  --body-file <path>       read the review body from a file (wins over -b)
 ```
 ```bash
 stoke pr review -o heavy-duty -r stoke -n 3 --event approve -b "Ship it."
 stoke pr review -o heavy-duty -r stoke -n 3 --event request-changes --body-file notes.md
 ```
-Calls `POST /api/v1/repos/{owner}/{repo}/pulls/{number}/reviews`.
+Calls `POST /api/v1/repos/{owner}/{repo}/pulls/{number}/reviews`. Prints the review URL when the forge returns one.
 ### `stoke branch list`
--- a/package-lock.json
+++ b/package-lock.json
@ -1,12 +1,12 @@
 {
  "name": "stoke",
-  "version": "1.2.0",
+  "version": "1.2.1",
  "lockfileVersion": 3,
  "requires": true,
  "packages": {
    "": {
      "name": "stoke",
-      "version": "1.2.0",
+      "version": "1.2.1",
      "license": "ISC",
      "dependencies": {
        "commander": "^15.0.0"
--- a/package.json
+++ b/package.json
@ -1,6 +1,6 @@
 {
  "name": "stoke",
-  "version": "1.2.0",
+  "version": "1.2.1",
  "description": "CLI for the heavy-duty forge (Forgejo)",
  "main": "src/cli.js",
  "scripts": {
--- a/scripts/install-apt.sh
+++ b/scripts/install-apt.sh
@ -18,9 +18,12 @@ FORGE_URL="${FORGE_URL:-https://forgejo.heavyduty.builders}"
 OWNER="${OWNER:-heavy-duty}"
 DISTRIBUTION="${DISTRIBUTION:-stable}"
 COMPONENT="${COMPONENT:-main}"
 # Where apt configuration lives; overridable so tests can run against a
 # throwaway directory instead of the real /etc/apt.
 APT_ETC="${STOKE_APT_ETC:-/etc/apt}"
-KEYRING="/etc/apt/keyrings/forgejo-$OWNER.asc"
+KEYRING="$APT_ETC/keyrings/forgejo-$OWNER.asc"
-LIST="/etc/apt/sources.list.d/forgejo-$OWNER.list"
+LIST="$APT_ETC/sources.list.d/forgejo-$OWNER.list"
 SUDO=""
 if [ "$(id -u)" -ne 0 ]; then
@ -28,25 +31,69 @@ if [ "$(id -u)" -ne 0 ]; then
  SUDO="sudo"
 fi
-update_only_this_source() {
+update_only_source() {
  $SUDO apt-get update \
-    -o Dir::Etc::sourcelist="$LIST" \
+    -o Dir::Etc::sourcelist="$1" \
    -o Dir::Etc::sourceparts=/dev/null \
    -o APT::Get::List-Cleanup=0
 }
 # stoke needs Node.js >= 22.12 (commander 15), but the distro archives of
 # Debian 13 (nodejs 20.x) and Ubuntu 24.04 (nodejs 18.x) cannot satisfy
 # that, which would make `apt-get install stoke` fail with an unmet
 # dependency. When no configured source offers a new-enough nodejs, add the
 # NodeSource repository for Node 22 so the dependency resolves.
 NODE_MIN="22.12"
 node_candidate_ok() {
  local candidate
  # LC_ALL=C: the "Candidate:" label is localized.
  candidate="$(LC_ALL=C apt-cache policy nodejs 2>/dev/null | sed -n 's/^  Candidate: //p')"
  [ -n "$candidate" ] && [ "$candidate" != "(none)" ] || return 1
  dpkg --compare-versions "${candidate#*:}" ge "$NODE_MIN"
 }
 ensure_nodejs_source() {
  node_candidate_ok && return 0
  # The verdict may just be stale package lists — refresh (best effort, a
  # transient failure of an unrelated source must not abort) and re-check
  # before adding anything.
  echo "No apt source seems to provide nodejs >= $NODE_MIN; refreshing apt metadata ..."
  $SUDO apt-get update || true
  node_candidate_ok && return 0
  local ns_keyring="$APT_ETC/keyrings/nodesource.asc"
  local ns_list="$APT_ETC/sources.list.d/nodesource.list"
  if [ -e "$ns_list" ]; then
    echo "error: even after refreshing apt metadata, no source provides nodejs >= $NODE_MIN," >&2
    echo "and $ns_list already exists; refusing to overwrite it." >&2
    echo "Point it at a Node >= 22 release (e.g. node_22.x) and re-run." >&2
    exit 1
  fi
  echo "Adding NodeSource (Node 22) ..."
  curl -fsSL https://deb.nodesource.com/gpgkey/nodesource-repo.gpg.key | $SUDO tee "$ns_keyring" >/dev/null
  echo "deb [signed-by=$ns_keyring] https://deb.nodesource.com/node_22.x nodistro main" \
    | $SUDO tee "$ns_list" >/dev/null
  # tee inherits our umask; apt's unprivileged _apt user must be able to
  # read these.
  $SUDO chmod 0644 "$ns_keyring" "$ns_list"
  update_only_source "$ns_list"
  node_candidate_ok || { echo "error: still no nodejs >= $NODE_MIN available after adding NodeSource" >&2; exit 1; }
 }
 echo "Adding APT source for $FORGE_URL/$OWNER ..."
-$SUDO install -d -m 0755 /etc/apt/keyrings
+$SUDO install -d -m 0755 "$APT_ETC/keyrings"
 curl -fsSL "$FORGE_URL/api/packages/$OWNER/debian/repository.key" | $SUDO tee "$KEYRING" >/dev/null
 echo "deb [signed-by=$KEYRING] $FORGE_URL/api/packages/$OWNER/debian $DISTRIBUTION $COMPONENT" \
  | $SUDO tee "$LIST" >/dev/null
 # tee inherits our umask; apt's unprivileged _apt user must be able to
 # read these.
 $SUDO chmod 0644 "$KEYRING" "$LIST"
 # Newer apt verifies with sqv (Sequoia), which rejects the signature Forgejo
 # currently produces for its Debian registry (malformed Ed25519 MPI encoding
 # in the upstream signing library). Try the properly signed source first so
 # this heals automatically once the forge is fixed; otherwise fall back to
 # [trusted=yes] — package integrity then relies on HTTPS to our own forge.
-if ! update_only_this_source; then
+if ! update_only_source "$LIST"; then
  echo
  echo "WARNING: signature verification failed (known Forgejo registry issue" >&2
  echo "with sqv-based apt). Falling back to [trusted=yes]; transport" >&2
@ -54,9 +101,12 @@ if ! update_only_this_source; then
  echo
  echo "deb [trusted=yes] $FORGE_URL/api/packages/$OWNER/debian $DISTRIBUTION $COMPONENT" \
    | $SUDO tee "$LIST" >/dev/null
-  update_only_this_source
+  $SUDO chmod 0644 "$LIST"
  update_only_source "$LIST"
 fi
 ensure_nodejs_source
 $SUDO apt-get install -y stoke
 echo
--- a/src/cli.js
+++ b/src/cli.js
@ -679,6 +679,20 @@ pr
    }
  });
 // Map CLI review event names (and common Forgejo ReviewStateType tokens) to
 // the values this forge accepts on POST .../pulls/{n}/reviews.
 const REVIEW_EVENT_MAP = {
  approve: 'APPROVED',
  approved: 'APPROVED',
  'request-changes': 'REQUEST_CHANGES',
  request_changes: 'REQUEST_CHANGES',
  comment: 'COMMENT',
 };
 function resolveReviewEvent(raw) {
  return REVIEW_EVENT_MAP[String(raw).toLowerCase()] || null;
 }
 pr
  .command('show')
  .description('Show details of a pull request')
@ -690,11 +704,15 @@ pr
      const config = loadConfig();
      const client = ForgejoClient.fromConfig(config);
      const prData = await client.getPullRequest(options.owner, options.repo, options.number);
      const author = prData.user?.login || '(unknown)';
      const headRef = prData.head?.ref || '?';
      const baseRef = prData.base?.ref || '?';
      const mergeable = prData.mergeable == null ? 'unknown' : String(prData.mergeable);
      console.log(`!${prData.number} [${prData.state}] ${prData.title}`);
      console.log(`URL:        ${prData.html_url}`);
-      console.log(`Author:     ${prData.user.login}`);
+      console.log(`Author:     ${author}`);
-      console.log(`Branch:     ${prData.head.ref} -> ${prData.base.ref}`);
+      console.log(`Branch:     ${headRef} -> ${baseRef}`);
-      console.log(`Mergeable:  ${prData.mergeable}`);
+      console.log(`Mergeable:  ${mergeable}`);
      console.log(`Created:    ${prData.created_at}`);
      if (prData.body) {
        console.log('\n' + prData.body);
@ -712,8 +730,8 @@ pr
  .requiredOption('-o, --owner <owner>', 'repository owner')
  .requiredOption('-r, --repo <repo>', 'repository name')
  .requiredOption('-n, --number <number>', 'pull request number', parseId)
-  .option('-b, --body <body>', 'comment body (markdown)')
+  .option('-b, --body <body>', 'comment body (markdown; required unless --body-file)')
-  .option('--body-file <path>', 'read the comment body from a file')
+  .option('--body-file <path>', 'read the comment body from a file (wins over -b)')
  .action(async (options) => {
    try {
      const config = loadConfig();
@ -739,22 +757,16 @@ pr
  .requiredOption('-o, --owner <owner>', 'repository owner')
  .requiredOption('-r, --repo <repo>', 'repository name')
  .requiredOption('-n, --number <number>', 'pull request number', parseId)
-  .requiredOption('--event <event>', 'review event: approve, request-changes|request_changes, comment')
+  .requiredOption('--event <event>', 'review event: approve|approved, request-changes|request_changes, comment')
-  .option('-b, --body <body>', 'review body (markdown)')
+  .option('-b, --body <body>', 'review body (markdown; required for request-changes and comment)')
-  .option('--body-file <path>', 'read the review body from a file')
+  .option('--body-file <path>', 'read the review body from a file (wins over -b)')
  .action(async (options) => {
    try {
      const config = loadConfig();
      const client = ForgejoClient.fromConfig(config);
-      const eventMap = {
+      const event = resolveReviewEvent(options.event);
        approve: 'APPROVED',
        'request-changes': 'REQUEST_CHANGES',
        request_changes: 'REQUEST_CHANGES',
        comment: 'COMMENT',
      };
      const event = eventMap[options.event.toLowerCase()];
      if (!event) {
-        console.error(`Invalid review event: ${options.event}. Must be approve, request-changes (or request_changes), or comment.`);
+        console.error(`Invalid review event: ${options.event}. Must be approve (or approved), request-changes (or request_changes), or comment.`);
        process.exit(1);
      }
      const rawBody = readBodyOption(options) || '';
@ -762,8 +774,11 @@ pr
        console.error(`Review event ${options.event} requires a non-empty body. Use -b/--body or --body-file.`);
        process.exit(1);
      }
-      await client.createPullRequestReview(options.owner, options.repo, options.number, event, rawBody);
+      const result = await client.createPullRequestReview(options.owner, options.repo, options.number, event, rawBody);
      console.log(`Review submitted on !${options.number}: ${event}.`);
      if (result && result.html_url) {
        console.log(`URL: ${result.html_url}`);
      }
    } catch (err) {
      console.error(`Failed to submit review: ${err.message}`);
      if (err.status) console.error(`HTTP status: ${err.status}`);
--- a/test/cli.test.js
+++ b/test/cli.test.js
@ -101,6 +101,18 @@ test('pr comment rejects a missing body before any network call', () => {
  }
 });
 test('pr comment rejects a whitespace-only body before any network call', () => {
  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
  fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
  try {
    const res = run(['pr', 'comment', '-o', 'o', '-r', 'r', '-n', '1', '-b', '   '], { STOKE_CONFIG_FILE: cfg });
    assert.equal(res.status, 1);
    assert.match(res.stderr, /Comment body is required/);
  } finally {
    fs.unlinkSync(cfg);
  }
 });
 test('pr review rejects an invalid event before any network call', () => {
  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
  fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
@ -113,6 +125,20 @@ test('pr review rejects an invalid event before any network call', () => {
  }
 });
 test('pr review accepts approved as an alias for approve before any network call', () => {
  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
  fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
  try {
    // Network fails; must not fail at event validation.
    const res = run(['pr', 'review', '-o', 'o', '-r', 'r', '-n', '1', '--event', 'APPROVED'], { STOKE_CONFIG_FILE: cfg });
    assert.equal(res.status, 1);
    assert.doesNotMatch(res.stderr, /Invalid review event/);
    assert.doesNotMatch(res.stderr, /requires a non-empty body/);
  } finally {
    fs.unlinkSync(cfg);
  }
 });
 test('pr review request-changes rejects a missing body before any network call', () => {
  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}.json`);
  fs.writeFileSync(cfg, JSON.stringify({ url: 'https://forge.test', token: 'tok' }));
@ -173,18 +199,29 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
  fs.writeFileSync(bodyFile, rawBody, 'utf8');
  const TIMEOUT_MS = 5000;
  let timer;
  const captured = await new Promise((resolve, reject) => {
    const fail = (err) => {
      clearTimeout(timer);
      try { server.close(); } catch { /* already closed */ }
      reject(err instanceof Error ? err : new Error(String(err)));
    };
    const server = http.createServer((req, res) => {
      let data = '';
      req.setEncoding('utf8');
      req.on('data', (chunk) => { data += chunk; });
      req.on('end', () => {
        res.writeHead(200, { 'Content-Type': 'application/json' });
-        res.end(JSON.stringify({ id: 99 }));
+        res.end(JSON.stringify({ id: 99, html_url: 'https://forge.test/reviews/99' }));
-        server.close(() => resolve({ url: req.url, body: data }));
+        clearTimeout(timer);
        server.close(() => resolve({ url: req.url, body: data, cliStatus: null }));
      });
    });
    timer = setTimeout(() => fail(new Error(`CLI boundary test timed out after ${TIMEOUT_MS}ms`)), TIMEOUT_MS);
    server.listen(0, '127.0.0.1', async () => {
      const { port } = server.address();
      fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' }));
@ -194,13 +231,16 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
          { STOKE_CONFIG_FILE: cfg },
        );
        if (res.status !== 0) {
-          server.close(() => reject(new Error(`CLI failed: ${res.stderr}`)));
+          fail(new Error(`CLI failed (status ${res.status}): ${res.stderr}`));
          return;
        }
        // Capture is resolved from the HTTP handler; assert exit 0 here via side channel.
        // If the handler already resolved, attach status for the outer asserts.
      } catch (err) {
-        server.close(() => reject(err));
+        fail(err);
      }
    });
-  });
+  }).finally(() => clearTimeout(timer));
  try {
    assert.equal(captured.url, '/api/v1/repos/o/r/pulls/7/reviews');
@ -212,3 +252,47 @@ test('pr review preserves exact body-file whitespace through the CLI boundary',
    fs.unlinkSync(bodyFile);
  }
 });
 test('pr review prints the review URL from the API response', async () => {
  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}-url.json`);
  const TIMEOUT_MS = 5000;
  let timer;
  const result = await new Promise((resolve, reject) => {
    const fail = (err) => {
      clearTimeout(timer);
      try { server.close(); } catch { /* already closed */ }
      reject(err instanceof Error ? err : new Error(String(err)));
    };
    const server = http.createServer((req, res) => {
      let data = '';
      req.on('data', (c) => { data += c; });
      req.on('end', () => {
        res.writeHead(200, { 'Content-Type': 'application/json' });
        res.end(JSON.stringify({ id: 42, html_url: 'https://forge.test/pulls/7#issuecomment-42' }));
      });
    });
    timer = setTimeout(() => fail(new Error('timeout')), TIMEOUT_MS);
    server.listen(0, '127.0.0.1', async () => {
      const { port } = server.address();
      fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' }));
      try {
        const res = await spawnAsync(
          ['pr', 'review', '-o', 'o', '-r', 'r', '-n', '7', '--event', 'approve', '-b', 'LGTM'],
          { STOKE_CONFIG_FILE: cfg },
        );
        clearTimeout(timer);
        server.close(() => resolve(res));
      } catch (err) {
        fail(err);
      }
    });
  }).finally(() => clearTimeout(timer));
  try {
    assert.equal(result.status, 0, result.stderr);
    assert.match(result.stdout, /Review submitted on !7: APPROVED/);
    assert.match(result.stdout, /URL: https:\/\/forge\.test\/pulls\/7#issuecomment-42/);
  } finally {
    fs.unlinkSync(cfg);
  }
 });
--- a/test/install-apt.test.js
+++ b/test/install-apt.test.js
@ -0,0 +1,149 @@
 const { test } = require('node:test');
 const assert = require('node:assert/strict');
 const { spawnSync } = require('node:child_process');
 const fs = require('node:fs');
 const os = require('node:os');
 const path = require('node:path');
 const SCRIPT = path.join(__dirname, '..', 'scripts', 'install-apt.sh');
 // Runs install-apt.sh against a throwaway apt directory (STOKE_APT_ETC) with
 // every external command stubbed via PATH. Scenario knobs:
 //   candInitial        `apt-cache policy` Candidate before any update
 //   candAfterUpdate    Candidate after any `apt-get update`
 //   candAfterNodesource Candidate after an update once nodesource.list exists
 // The apt-cache stub localizes the "Candidate:" label unless LC_ALL=C is set,
 // so every scenario doubles as a regression test for locale-safe parsing.
 const cleanups = [];
 process.on('exit', () => { for (const dir of cleanups) fs.rmSync(dir, { recursive: true, force: true }); });
 function runScenario({ candInitial, candAfterUpdate, candAfterNodesource, preexistingNodesourceList }) {
  const root = fs.mkdtempSync(path.join(os.tmpdir(), 'stoke-apt-test-'));
  cleanups.push(root);
  const bin = path.join(root, 'bin');
  const state = path.join(root, 'state');
  const aptEtc = path.join(root, 'etc', 'apt');
  fs.mkdirSync(bin, { recursive: true });
  fs.mkdirSync(state, { recursive: true });
  fs.mkdirSync(path.join(aptEtc, 'sources.list.d'), { recursive: true });
  fs.writeFileSync(path.join(state, 'candidate'), candInitial);
  if (preexistingNodesourceList !== undefined) {
    fs.writeFileSync(path.join(aptEtc, 'sources.list.d', 'nodesource.list'), preexistingNodesourceList);
  }
  const stub = (name, body) => {
    const p = path.join(bin, name);
    fs.writeFileSync(p, `#!/usr/bin/env bash\n${body}\n`, { mode: 0o755 });
  };
  // Force the non-root path so every mutation goes through the sudo stub.
  stub('id', 'echo 1000');
  stub('sudo', 'exec "$@"');
  stub('curl', 'echo "FAKE-KEY"');
  stub('stoke', 'echo 1.2.0');
  stub('apt-cache', [
    'cand="$(cat "$STATE_DIR/candidate")"',
    '[ "$cand" = "absent" ] && exit 0',
    'label="Candidato"',
    '[ "${LC_ALL:-}" = "C" ] && label="Candidate"',
    'printf "nodejs:\\n  Installed: (none)\\n  %s: %s\\n" "$label" "$cand"',
  ].join('\n'));
  stub('apt-get', [
    'echo "apt-get $*" >> "$STATE_DIR/apt-get.log"',
    'for a in "$@"; do',
    '  if [ "$a" = update ]; then',
    '    if [ -e "$STOKE_APT_ETC/sources.list.d/nodesource.list" ] && [ -n "${CAND_AFTER_NODESOURCE:-}" ]; then',
    '      echo "$CAND_AFTER_NODESOURCE" > "$STATE_DIR/candidate"',
    '    elif [ -n "${CAND_AFTER_UPDATE:-}" ]; then',
    '      echo "$CAND_AFTER_UPDATE" > "$STATE_DIR/candidate"',
    '    fi',
    '  fi',
    'done',
    'exit 0',
  ].join('\n'));
  // Restrictive umask: apt-readable 0644 files must come from the script's
  // explicit chmod, not from a lucky default.
  const res = spawnSync('bash', ['-c', 'umask 077 && exec bash "$1"', 'bash', SCRIPT], {
    encoding: 'utf8',
    env: {
      ...process.env,
      PATH: `${bin}:${process.env.PATH}`,
      STOKE_APT_ETC: aptEtc,
      STATE_DIR: state,
      CAND_AFTER_UPDATE: candAfterUpdate || '',
      CAND_AFTER_NODESOURCE: candAfterNodesource || '',
      LC_ALL: 'es_ES.UTF-8', // localized environment; the script must force C
    },
  });
  const read = (p) => (fs.existsSync(p) ? fs.readFileSync(p, 'utf8') : null);
  const mode = (p) => (fs.existsSync(p) ? fs.statSync(p).mode & 0o777 : null);
  const result = {
    res,
    aptEtc,
    nodesourceList: read(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
    nodesourceListMode: mode(path.join(aptEtc, 'sources.list.d', 'nodesource.list')),
    nodesourceKey: read(path.join(aptEtc, 'keyrings', 'nodesource.asc')),
    nodesourceKeyMode: mode(path.join(aptEtc, 'keyrings', 'nodesource.asc')),
    forgeKeyMode: mode(path.join(aptEtc, 'keyrings', 'forgejo-heavy-duty.asc')),
    aptGetLog: read(path.join(state, 'apt-get.log')) || '',
  };
  // Drop the throwaway tree after we have read everything we need.
  fs.rmSync(root, { recursive: true, force: true });
  return result;
 }
 test('suitable nodejs candidate already available: installs without touching NodeSource', () => {
  // Epoch-prefixed version also covers the epoch-stripping in the comparison.
  const s = runScenario({ candInitial: '1:22.23.1-1nodesource1' });
  assert.equal(s.res.status, 0, s.res.stderr);
  assert.equal(s.nodesourceList, null);
  assert.match(s.aptGetLog, /install -y stoke/);
  assert.equal(s.forgeKeyMode, 0o644, 'forge keyring must be readable by _apt');
 });
 test('no cached metadata: refreshes apt lists before deciding, no NodeSource needed', () => {
  const s = runScenario({ candInitial: 'absent', candAfterUpdate: '22.23.1-1nodesource1' });
  assert.equal(s.res.status, 0, s.res.stderr);
  assert.equal(s.nodesourceList, null);
  assert.match(s.aptGetLog, /install -y stoke/);
 });
 test('distro nodejs too old: bootstraps NodeSource and installs', () => {
  const s = runScenario({
    candInitial: '20.19.2+dfsg-1+deb13u2',
    candAfterUpdate: '20.19.2+dfsg-1+deb13u2',
    candAfterNodesource: '22.23.1-1nodesource1',
  });
  assert.equal(s.res.status, 0, s.res.stderr);
  assert.match(s.nodesourceList, /deb \[signed-by=.*nodesource\.asc\] https:\/\/deb\.nodesource\.com\/node_22\.x nodistro main/);
  assert.equal(s.nodesourceKey, 'FAKE-KEY\n');
  assert.equal(s.nodesourceKeyMode, 0o644, 'NodeSource keyring must be readable by _apt');
  assert.equal(s.nodesourceListMode, 0o644, 'NodeSource list must be readable by _apt');
  assert.match(s.aptGetLog, /install -y stoke/);
 });
 test('bootstrap failure: NodeSource still lacks a suitable nodejs, exits with error', () => {
  const s = runScenario({
    candInitial: '20.19.2+dfsg-1+deb13u2',
    candAfterUpdate: '20.19.2+dfsg-1+deb13u2',
    candAfterNodesource: '20.19.2+dfsg-1+deb13u2',
  });
  assert.notEqual(s.res.status, 0);
  assert.match(s.res.stderr, /still no nodejs >= 22\.12/);
  assert.doesNotMatch(s.aptGetLog, /install -y stoke/);
 });
 test('pre-existing user-managed nodesource.list is never overwritten', () => {
  const marker = '# user-managed entry\n';
  const s = runScenario({
    candInitial: '18.19.1+dfsg-6ubuntu5',
    candAfterUpdate: '18.19.1+dfsg-6ubuntu5',
    preexistingNodesourceList: marker,
  });
  assert.notEqual(s.res.status, 0);
  assert.match(s.res.stderr, /refusing to overwrite/);
  assert.equal(s.nodesourceList, marker);
  assert.doesNotMatch(s.aptGetLog, /install -y stoke/);
 });
Author	SHA1	Message	Date
kimi-reviewer-andresmgsl	7deab688a2	Fix install-apt test return after rebase	2026-07-23 00:08:06 +00:00
grok-reviewer-andresmgsl	d959d2a6f4	Polish PR review CLI and harden install-apt (v1.2.1) - Accept approve/approved review event aliases; print review html_url - Harden pr show against missing user/head/base; clarify body-file wins - Add tests for whitespace-only comments, approved alias, review URL - Timeout the CLI-boundary HTTP fixture; clean up install-apt test trees - Clearer refuse-to-overwrite message when nodesource.list already exists - Merge Node 22 NodeSource bootstrap (from fix/apt-nodejs-bootstrap)	2026-07-23 00:06:15 +00:00
kimi-reviewer-andresmgsl	41b65a2bbd	Merge pull request 'install-apt: bootstrap Node 22 when distro nodejs cannot satisfy the dependency' (#5 ) from fix/apt-nodejs-bootstrap into main	2026-07-22 23:19:04 +00:00
cluade-reviewer-andresmgsl	0ecd935528	install-apt: address review nits — apt-readable file modes, test cleanup, clearer refusal error - chmod 0644 every keyring and sources.list entry after writing: tee inherits the caller's umask, and under e.g. umask 077 apt's unprivileged _apt user could not read the keyring - Tests now run the script under umask 077 and assert the 0644 modes (mutation-checked: dropping the chmod fails a test), and remove their temp directories on exit - Refusal error now states that metadata was already refreshed before concluding the existing nodesource.list is unsuitable Verified on fresh debian:13 under umask 077: all four files 0644, install succeeds. npm test 32/32. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 23:17:20 +00:00
cluade-reviewer-andresmgsl	444470301c	test: stubbed shell tests for the NodeSource bootstrap logic Covers the scenarios codex-reviewer recommended: suitable candidate already present (incl. epoch stripping), missing metadata healed by a refresh, bootstrap on too-old distro nodejs, bootstrap failure, and the refuse-to-overwrite branch for a user-managed nodesource.list. Every scenario runs under a localized LC_ALL with an apt-cache stub that only emits the English Candidate: label under LC_ALL=C, so locale-safe parsing is regression-tested (mutation-checked: dropping LC_ALL=C fails 3 tests). install-apt.sh gains STOKE_APT_ETC to redirect /etc/apt to a throwaway directory under test, following the script's existing env-override pattern. Real-container flow re-verified on debian:13. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 22:01:51 +00:00
cluade-reviewer-andresmgsl	5e99006d04	install-apt: address review — locale-safe parsing, metadata refresh, no list clobber, README order - Parse apt-cache policy under LC_ALL=C (Candidate: label is localized) - Refresh apt metadata (best effort) and re-check before concluding no suitable nodejs source exists - Refuse to overwrite an existing /etc/apt/sources.list.d/nodesource.list instead of silently replacing a user-managed entry - README: manual path now adds the forge source, then the Node 22 source, then runs apt-get update && install — in that order Verified on fresh debian:13: install, idempotent re-run (NodeSource not re-added), and the refusal branch with a pre-existing user list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 21:56:26 +00:00
cluade-reviewer-andresmgsl	0b4947b038	install-apt: bootstrap Node 22 via NodeSource when distro nodejs is too old (#1 ) The package depends on nodejs (>= 22.12), but Debian 13 ships Node 20 and Ubuntu 24.04 ships Node 18, so a fresh container failed apt-get install with an unmet dependency. install-apt.sh now checks whether any configured apt source can satisfy the requirement and, if not, adds the NodeSource Node 22 repository before installing. README documents the behaviour and the manual equivalent. Verified on fresh debian:13 and ubuntu:24.04 containers: one-line setup, apt-get install stoke, stoke --version all succeed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>	2026-07-22 21:39:51 +00:00