fix(worktree): the npm-install log moves into the worktree's own gitdir

[Brad (bmethod)]

The npm-install log moves into the worktree's own gitdir

Completes the follow-up promised on PR #1451's review threads: two reviewers showed the ${TMPDIR:-/tmp}/worktree-npm-install-$(id -u) scheme kept a raceable shared-tmp surface (log-file symlink pre-plant; non-atomic -L/-d/-O/chmod sequences on the directory). Rather than another hardening rung, the surface is deleted: the failed-install log now lives at <gitdir>/npm-install.log — each linked worktree's private gitdir under the main checkout's .git/worktrees/<name>/, owner-controlled and per-worktree by construction. No shared directory, no predictable /tmp leaf, no check-then-use window.

• Unresolvable gitdir or unwritable path → the install runs unlogged (probed with : > so a bad path falls through instead of aborting before npm runs).
• Unchanged: detached std fds (a caller capturing $(create) gets EOF immediately), rm -f -- "$log" on success, failed install keeps its full log.
• README provisioning paragraph documents the new location.
• The worktree script shrank; its baseline row tightened 4149 → 4143.

Tests

14/14 green (the 12-pin dependency-install suite + 2 new pins: a failed install leaves its log at the gitdir path; a successful one leaves none). Both new pins have verified must-fail controls: forcing gitdir resolution empty fails the log pin, removing the success-path rm fails the cleanup pin. Full tools/validate-changed chain exits 0.

https://claude.ai/code/session_01EwxSRv8oy1NxoQm66WKa4J

[qodo-code-review]

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

[macroscopeapp]

Approvability

Verdict: Approved af1bfea

This change relocates npm-install logs from a shared temp directory to the worktree's own gitdir, simplifying security handling while keeping the same install behavior. The change is self-contained, includes comprehensive tests, and reduces code complexity.

No code changes detected at 5cef82d. Prior analysis still applies.

^{You can customize Macroscope's approvability policy. Learn more.}

[Copilot]

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Updates npm install logging for worktrees to use a per-worktree gitdir log file, and adds coverage to ensure logs are retained/cleared appropriately.

Changes:

• Move npm install logging from a shared ${TMPDIR} directory to <gitdir>/npm-install.log per worktree.
• Extend shell test coverage to validate log retention on failure and log cleanup on success.
• Update documentation and size ratchet baseline to reflect the new behavior/size.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
tools/size-ratchet-baseline.tsv	Updates the recorded size baseline for the worktree script after the logging change.
skills/worktree/tests/worktree_js_dependency_install.sh	Adds assertions and helpers to check that install logs are removed on success and retained on failure.
skills/worktree/scripts/worktree	Changes npm install logging location to the worktree’s gitdir and cleans it up on success.
skills/worktree/README.md	Documents the new npm install log location and behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c9f9e3eb0f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

Dismissing prior approval to re-evaluate 0784ea4

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (4)

skills/worktree/scripts/worktree:1803

• umask 077 is set before running npm install, which will also affect permissions of everything npm creates (e.g., node_modules, caches, generated scripts). If the intent is only to make npm-install.log owner-only, capture the prior umask, set 077 just for log-file creation, then restore the original umask before invoking npm install.

      umask 077
      log=""
      if gitdir="$(git rev-parse --absolute-git-dir 2>/dev/null)" &&
        [ -n "$gitdir" ] && [ -d "$gitdir" ] &&
        rm -f -- "$gitdir/npm-install.log" 2>/dev/null &&
        : >"$gitdir/npm-install.log" 2>/dev/null; then
        log="$gitdir/npm-install.log"
      fi
      if [ -n "$log" ]; then
        if npm install --no-audit --no-fund >"$log" 2>&1; then
          rm -f -- "$log"
        fi

skills/worktree/tests/worktree_js_dependency_install.sh:269

• With set -e, a failing install_log_path inside command substitution can terminate the test script before bad runs (depending on Bash’s errexit behavior). To keep failures reported via bad instead of aborting the harness, resolve the path into a variable with an explicit fallback (e.g., log=$(install_log_path ... || echo '<unresolved>')) and pass that variable to bad.

  bad "clean install clears its log" "$(install_log_path "$WT_NPM") still exists"

skills/worktree/tests/worktree_js_dependency_install.sh:285

• With set -e, a failing install_log_path inside command substitution can terminate the test script before bad runs (depending on Bash’s errexit behavior). To keep failures reported via bad instead of aborting the harness, resolve the path into a variable with an explicit fallback (e.g., log=$(install_log_path ... || echo '<unresolved>')) and pass that variable to bad.

  bad "failed install log" "$(install_log_path "$WT_FAIL") missing, empty, or truncated"

skills/worktree/scripts/worktree:1796

• The rm -f followed by : > file is redundant for truncation/creation. If you primarily want to validate writability up-front, consider collapsing this to a single truncation/create operation (and keep any explicit existence/permission checks you need), which simplifies the control flow and reduces filesystem ops.

        rm -f -- "$gitdir/npm-install.log" 2>/dev/null &&
        : >"$gitdir/npm-install.log" 2>/dev/null; then

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0784ea4fda

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "Codex (@codex) review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "Codex (@codex) address that feedback".

[github-actions]

Merge queue ejected this PR (removed_from_merge_queue, reason: MANUAL). The auto-merge arm is now DROPPED — nothing will merge this PR until someone re-arms it.

Ejecting merge-group run: not identified

Failing job(s):
(failing run not identified — the dequeue may predate run completion; check the queue insights page)

No same-named check comparison available for the PR head.

Automated by merge-queue-ejection-alert (VST-196). This alert never re-arms auto-merge.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

skills/worktree/tests/worktree_js_dependency_install.sh:270

• wait_for_install_log_removal returns success immediately if the log file does not exist, which can produce a false positive (the test can pass before the background install ever creates/removes the log). To make this deterministic, first wait for the log to be created (or for some positive signal that install started), then assert it gets removed; alternatively, gate the removal check on having observed the log at least once during the wait window.

WT_NPM="$ROOT/.worktrees/repo/issue-npm"
if wait_for_install_log_removal "$WT_NPM"; then
  ok "a clean install leaves no npm-install.log behind"
else
  bad "clean install clears its log" "$(install_log_path "$WT_NPM") still exists"
fi

Dismissing prior approval to re-evaluate af1bfea

[github-actions]

Merge queue ejected this PR (removed_from_merge_queue, reason: MERGE_CONFLICT). The auto-merge arm is now DROPPED — nothing will merge this PR until someone re-arms it.

Ejecting merge-group run: not identified

Failing job(s):
(failing run not identified — the dequeue may predate run completion; check the queue insights page)

No same-named check comparison available for the PR head.

Automated by merge-queue-ejection-alert (VST-196). This alert never re-arms auto-merge.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

skills/worktree/tests/worktree_js_dependency_install.sh:269

• This test runs with set -e, and install_log_path is called inside command substitution in the failure-message path. If install_log_path itself fails (e.g., gitdir can’t be resolved), errexit can terminate the entire test early, skipping the intended bad reporting and final PASS/FAIL summary. Prefer computing the path with an error-tolerant assignment before the if (or guard the substitution with || true / a fallback string) so failures are reported via bad rather than aborting the script.

  bad "clean install clears its log" "$(install_log_path "$WT_NPM") still exists"

skills/worktree/tests/worktree_js_dependency_install.sh:285

• This test runs with set -e, and install_log_path is called inside command substitution in the failure-message path. If install_log_path itself fails (e.g., gitdir can’t be resolved), errexit can terminate the entire test early, skipping the intended bad reporting and final PASS/FAIL summary. Prefer computing the path with an error-tolerant assignment before the if (or guard the substitution with || true / a fallback string) so failures are reported via bad rather than aborting the script.

  bad "failed install log" "$(install_log_path "$WT_FAIL") missing, empty, or truncated"