From 1d8adadf2c0949da57ed41849d61c92667eca712 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Sun, 11 Oct 2026 18:38:36 +0200 Subject: [PATCH] feat: install a JavaScript workspace's dependencies before the analysis With install_dependencies (default true), the action asks Core to install a JavaScript workspace's dependencies, so calls between its packages resolve. Core checks the head and the merge base each on its own and fetches the package manager each one pins; the action only turns it on and keeps the package managers' downloads in one cache, saved by sync. - never for a fork's pull request: its package manager would run with the job's credentials - an install is part of the analysis identity (cfg hash), so an analysis made without one is never reused by a run that installs, or the other way round Co-Authored-By: Claude Opus 5.5 --- README.md | 10 +++ action.yml | 36 +++++++++ scripts/action/prepare-dependencies.sh | 37 +++++++++ scripts/action/resolve-cache-keys.sh | 6 +- tests/test_action_state.py | 4 + tests/test_prepare_dependencies.py | 106 +++++++++++++++++++++++++ 6 files changed, 197 insertions(+), 2 deletions(-) create mode 100755 scripts/action/prepare-dependencies.sh create mode 100644 tests/test_prepare_dependencies.py diff --git a/README.md b/README.md index 922386e..8ad568e 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,16 @@ A pull request whose changed files are all outside what the engine analyses (doc Every review comment ends with a machine-readable HTML comment, ``, for readers that should not parse the prose or the diagram. The progress comment carries the platform link from the start, so a pull request can be opened there while the run is still going. +### JavaScript workspaces + +In a JavaScript or TypeScript monorepo (npm, pnpm, Yarn or Bun workspaces), a call from one package into another only resolves once the packages are installed, so a fresh checkout shows no calls between them. With `install_dependencies: true` (the default), CodeBoarding installs the analyzed repository's dependencies before analyzing it, for the pull request and for its merge base alike: + +- **With the lockfile frozen and without dependency install scripts.** The repository's own package-manager setup (`.npmrc`, `.yarnrc.yml`) applies, as it would in a build. +- **With the package manager the repository pins** in `packageManager`, else the one its lockfile belongs to. When the runner lacks that version, it is fetched from the public npm registry, outside the repository, and reused by later runs on the same machine. +- **From a cached store.** Downloads are cached once per lockfile, about 1 GB for a large monorepo, against the repository's 10 GB of free Actions cache. Only sync saves it, so pull request runs read it without adding to the repository's cache use. + +A pull request from a fork is analyzed without an install: its package manager would run with the job's credentials in reach. A repository that is not a workspace, has no lockfile or uses Plug'n'Play (Yarn or pnpm) is analyzed without one too, and so is one whose install fails. For a private registry, set the token your `.npmrc` reads in the job's `env` (for example `NPM_TOKEN: ${{ secrets.NPM_TOKEN }}`). Set `install_dependencies: false` to skip installing entirely. + ## Authentication and providers The `llm` input is required and says where analysis credentials come from. There are diff --git a/action.yml b/action.yml index 33ef7d0..20a9a8e 100644 --- a/action.yml +++ b/action.yml @@ -153,6 +153,10 @@ inputs: in_place: committed as .codeboarding/ on the synced branch itself. required: false default: 'codeboarding_branch' + install_dependencies: + description: "Install the analyzed repository's JavaScript workspace dependencies before the analysis (no install scripts, frozen lockfile), so calls between its packages resolve. Never for a fork's pull request. Set to false to skip." + required: false + default: 'true' warmstart_retention_days: description: 'Days to keep the reusable analysis. Only the next run reads it.' required: false @@ -400,6 +404,28 @@ runs: distribution: temurin java-version: '21' + - name: Prepare dependency install + id: deps + if: steps.guard.outputs.skip != 'true' + shell: bash + env: + CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target + STORE_DIR: ${{ runner.temp }}/cb-deps + INSTALL_DEPENDENCIES: ${{ inputs.install_dependencies }} + IS_FORK: ${{ steps.guard.outputs.is_fork }} + run: "$GITHUB_ACTION_PATH/scripts/action/prepare-dependencies.sh" + + # Only sync saves the store, on the branch it runs on, which PR runs can read; a cache saved + # inside one pull request is visible to no other, so it would only use up the repository's quota. + - name: Restore dependency cache + id: deps_cache + if: steps.guard.outputs.skip != 'true' && steps.deps.outputs.lockfile == 'true' + uses: actions/cache/restore@v4 + with: + path: ${{ runner.temp }}/cb-deps + key: cb-deps-${{ runner.os }}-${{ hashFiles('.codeboarding-target/*.lock', '.codeboarding-target/*.lockb', '.codeboarding-target/*-lock.yaml', '.codeboarding-target/*-lock.json', '.codeboarding-target/npm-shrinkwrap.json') }} + restore-keys: cb-deps-${{ runner.os }}- + - name: Install CodeBoarding if: steps.guard.outputs.skip != 'true' shell: bash @@ -436,6 +462,8 @@ runs: MODEL: ${{ inputs.model }} AGENT_MODEL_INPUT: ${{ inputs.agent_model }} PARSING_MODEL_INPUT: ${{ inputs.parsing_model }} + # A fork's review installs nothing, so it never reuses an analysis made with an install. + INSTALL_DEPENDENCIES: ${{ steps.deps.outputs.lockfile == 'true' }} run: "$GITHUB_ACTION_PATH/scripts/action/resolve-cache-keys.sh" # Both lookups are best effort. Without them the run derives the base from @@ -547,6 +575,14 @@ runs: retention-days: 30 if-no-files-found: ignore + - name: Save dependency cache + if: steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' && steps.deps.outputs.lockfile == 'true' && steps.deps_cache.outputs.cache-hit != 'true' && steps.sync_analyze.outcome == 'success' + continue-on-error: true + uses: actions/cache/save@v4 + with: + path: ${{ runner.temp }}/cb-deps + key: ${{ steps.deps_cache.outputs.cache-primary-key }} + - name: Write sync summary if: always() && steps.guard.outputs.skip != 'true' && steps.guard.outputs.mode == 'sync' shell: bash diff --git a/scripts/action/prepare-dependencies.sh b/scripts/action/prepare-dependencies.sh new file mode 100755 index 0000000..70f9428 --- /dev/null +++ b/scripts/action/prepare-dependencies.sh @@ -0,0 +1,37 @@ +#!/usr/bin/env bash +# Turns on Core's install of a JavaScript workspace's dependencies and points every package +# manager's download cache at one store, which the dependency cache keeps between runs. +# +# Why Core decides the rest: it checks the head and the merge-base worktree each on its own, and +# fetches the package manager and version each one pins. Never for a fork's pull request: its +# package manager would run with this job's credentials in reach. +# +# Output: lockfile=true when the head has a lockfile, so the cache steps have something to key on. +set -euo pipefail + +if [ "${INSTALL_DEPENDENCIES:-true}" != true ]; then + echo "Dependencies: not installed (install_dependencies is false)." + exit 0 +fi +if [ "${IS_FORK:-false}" = true ]; then + echo "Dependencies: not installed (a fork's pull request)." + exit 0 +fi + +mkdir -p "$STORE_DIR" +{ + echo "CODEBOARDING_INSTALL_DEPENDENCIES=1" + echo "npm_config_cache=$STORE_DIR/npm" + echo "npm_config_store_dir=$STORE_DIR/pnpm" + echo "BUN_INSTALL_CACHE_DIR=$STORE_DIR/bun" + echo "YARN_CACHE_FOLDER=$STORE_DIR/yarn" + echo "YARN_GLOBAL_FOLDER=$STORE_DIR/yarn-berry" +} >> "$GITHUB_ENV" + +for name in bun.lock bun.lockb pnpm-lock.yaml yarn.lock package-lock.json npm-shrinkwrap.json; do + if [ -f "$CHECKOUT_DIR/$name" ]; then + echo "lockfile=true" >> "$GITHUB_OUTPUT" + break + fi +done +echo "Dependencies: Core installs a JavaScript workspace's dependencies, without install scripts." diff --git a/scripts/action/resolve-cache-keys.sh b/scripts/action/resolve-cache-keys.sh index b658ba6..feb7611 100755 --- a/scripts/action/resolve-cache-keys.sh +++ b/scripts/action/resolve-cache-keys.sh @@ -33,8 +33,10 @@ ignore_file="$CHECKOUT_DIR/.codeboarding/.codeboardingignore" # does. It carries no key: rotating a secret must not throw away reusable analysis. model_digest="$(printf '%s\n%s\n%s\n%s\n%s\n' \ "${LLM_PROVIDER:-}" "${BACKEND_ID:-}" "${MODEL:-}" "${AGENT_MODEL_INPUT:-}" "${PARSING_MODEL_INPUT:-}" | digest)" -cfg="$(printf '%s\n%s\n%s\n%s\n%s\n' \ - "$STATE_SCHEMA" "$engine_version" "$ignore_digest" "$model_digest" "${DEPTH_CAP:-2}" | digest)" +# Installed dependencies decide which calls between workspace packages resolve. +cfg="$(printf '%s\n%s\n%s\n%s\n%s\n%s\n' \ + "$STATE_SCHEMA" "$engine_version" "$ignore_digest" "$model_digest" "${DEPTH_CAP:-2}" \ + "${INSTALL_DEPENDENCIES:-false}" | digest)" { echo "engine_version=$engine_version" diff --git a/tests/test_action_state.py b/tests/test_action_state.py index 0a02ea4..fe51235 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -170,6 +170,10 @@ def test_analysis_scope_and_engine_version_change_the_identity(self) -> None: (self.checkout / ".codeboarding" / ".codeboardingignore").write_text("docs/\n", encoding="utf-8") self.assertNotEqual(baseline["cfg_hash"], self._run()["cfg_hash"]) + def test_installing_dependencies_changes_the_identity(self) -> None: + # Installed packages decide which calls between workspace packages resolve. + self.assertNotEqual(self._run()["cfg_hash"], self._run(INSTALL_DEPENDENCIES="true")["cfg_hash"]) + def test_model_selection_changes_the_identity(self) -> None: baseline = self._run() diff --git a/tests/test_prepare_dependencies.py b/tests/test_prepare_dependencies.py new file mode 100644 index 0000000..4e62de2 --- /dev/null +++ b/tests/test_prepare_dependencies.py @@ -0,0 +1,106 @@ +"""The dependency step turns Core's install on, except for a fork, and keys the cache on a lockfile.""" + +from __future__ import annotations + +import os +import re +import subprocess +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +SCRIPT = ROOT / "scripts" / "action" / "prepare-dependencies.sh" + + +def prepare(files: dict[str, str], install: str = "true", is_fork: str = "false") -> tuple[dict, dict]: + """Runs the step on a checkout holding *files*; returns its outputs and its exported env.""" + with tempfile.TemporaryDirectory() as tmp: + checkout = Path(tmp) / "checkout" + checkout.mkdir() + for name, text in files.items(): + (checkout / name).write_text(text, encoding="utf-8") + output, env_file = Path(tmp) / "output", Path(tmp) / "env" + output.write_text("", encoding="utf-8") + env_file.write_text("", encoding="utf-8") + subprocess.run( + [str(SCRIPT)], + env={ + "PATH": os.environ["PATH"], + "GITHUB_OUTPUT": str(output), + "GITHUB_ENV": str(env_file), + "CHECKOUT_DIR": str(checkout), + "STORE_DIR": str(Path(tmp) / "store"), + "INSTALL_DEPENDENCIES": install, + "IS_FORK": is_fork, + }, + check=True, + capture_output=True, + ) + outputs = dict(line.split("=", 1) for line in output.read_text(encoding="utf-8").splitlines()) + exported = dict(line.split("=", 1) for line in env_file.read_text(encoding="utf-8").splitlines()) + return outputs, exported + + +class PrepareDependenciesTests(unittest.TestCase): + def test_core_is_asked_to_install_and_every_cache_points_at_the_store(self) -> None: + outputs, env = prepare({"package.json": "{}", "pnpm-lock.yaml": ""}) + self.assertEqual(outputs, {"lockfile": "true"}) + self.assertEqual(env["CODEBOARDING_INSTALL_DEPENDENCIES"], "1") + store = env["npm_config_cache"].rsplit("/", 1)[0] + for variable in ("npm_config_store_dir", "BUN_INSTALL_CACHE_DIR", "YARN_CACHE_FOLDER", "YARN_GLOBAL_FOLDER"): + self.assertTrue(env[variable].startswith(store), variable) + + def test_without_a_lockfile_core_still_checks_the_merge_base(self) -> None: + outputs, env = prepare({"main.py": ""}) + self.assertEqual(outputs, {}) + self.assertEqual(env["CODEBOARDING_INSTALL_DEPENDENCIES"], "1") + + def test_a_forks_pull_request_installs_nothing(self) -> None: + self.assertEqual(prepare({"bun.lock": ""}, is_fork="true"), ({}, {})) + + def test_turning_it_off_installs_nothing(self) -> None: + self.assertEqual(prepare({"bun.lock": ""}, install="false"), ({}, {})) + + +class ActionWiringTests(unittest.TestCase): + """The steps around the script: the store is restored for every run and saved only by sync.""" + + def setUp(self) -> None: + self.action = (ROOT / "action.yml").read_text(encoding="utf-8") + + def _step(self, name: str) -> str: + return self.action.split(f"- name: {name}", 1)[1].split("- name:", 1)[0] + + def test_the_input_defaults_to_installing(self) -> None: + block = re.search(r"^ install_dependencies:\n((?: .*\n)+)", self.action, re.MULTILINE) + assert block is not None + self.assertIn("default: 'true'", block.group(1)) + + def test_the_step_knows_a_fork(self) -> None: + self.assertIn("IS_FORK: ${{ steps.guard.outputs.is_fork }}", self._step("Prepare dependency install")) + + def test_only_sync_saves_the_store(self) -> None: + save = self._step("Save dependency cache") + self.assertIn("steps.guard.outputs.mode == 'sync'", save) + self.assertIn("actions/cache/save@", save) + + def test_every_lockfile_keys_the_cache(self) -> None: + key = self._step("Restore dependency cache") + for pattern in ("*.lock", "*.lockb", "*-lock.yaml", "*-lock.json", "npm-shrinkwrap.json"): + self.assertIn(f"'.codeboarding-target/{pattern}'", key) + + def test_an_install_is_part_of_the_analysis_identity(self) -> None: + """A fork's review installs nothing, so it must not reuse a base analyzed with an install.""" + self.assertIn( + "INSTALL_DEPENDENCIES: ${{ steps.deps.outputs.lockfile == 'true' }}", + self._step("Resolve analysis cache keys"), + ) + + def test_the_store_is_ready_before_the_engine_runs(self) -> None: + for step in ("Generate sync baseline", "Generate review baseline"): + self.assertLess(self.action.index("- name: Restore dependency cache"), self.action.index(f"- name: {step}")) + + +if __name__ == "__main__": + unittest.main()