Skip to content

Reduce duplicate dependency storage across worktrees#3325

Draft
nighca wants to merge 1 commit into
goplus:devfrom
nighca:issue-3234
Draft

Reduce duplicate dependency storage across worktrees#3325
nighca wants to merge 1 commit into
goplus:devfrom
nighca:issue-3234

Conversation

@nighca

@nighca nighca commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Closes #3234

Summary

  • migrate frontend package installs from npm lockfiles to pnpm lockfiles
  • update CI, Docker, Vercel, docs, and local tasks to use pnpm
  • install SPX web assets through a shared user cache and link them into each git worktree
  • prune unused cached SPX versions by checking symlinks across git worktree list
  • add explicit pnpm dependencies/types needed under strict dependency resolution

Validation

  • corepack pnpm@11.9.0 install --frozen-lockfile in spx-gui
  • corepack pnpm@11.9.0 install --frozen-lockfile in tutorial
  • corepack pnpm@11.9.0 install --frozen-lockfile in ui/prototype
  • bash -n spx-gui/install-spx.sh && bash -n tutorial/build.sh && bash -n spx-gui/vercel-install.sh
  • bash spx-gui/install-spx.sh
  • SPX cache prune smoke test with an unused fake version
  • corepack pnpm@11.9.0 run type-check in spx-gui
  • corepack pnpm@11.9.0 run lint in spx-gui
  • corepack pnpm@11.9.0 run format-check in spx-gui
  • corepack pnpm@11.9.0 run test -- --run in spx-gui
  • bash build-wasm.sh in spx-gui
  • corepack pnpm@11.9.0 run build in spx-gui
  • corepack pnpm@11.9.0 run build:account in spx-gui
  • git diff --check

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request migrates the project's package manager from npm to pnpm, updating various Dockerfiles, scripts, documentation, and configuration files, while adding workspace and lock files. It also refactors install-spx.sh to implement a file-locking mechanism, caching, and version pruning, and adds explicit return types to several TypeScript utility functions. Feedback highlights that pnpm-workspace.yaml must be copied into the Docker builds to prevent installation failures, and recommends adding a timeout to the lock acquisition in install-spx.sh to avoid infinite hangs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread spx-gui/build/Dockerfile Outdated
WORKDIR /app/spx-gui

COPY spx-gui/package.json spx-gui/package-lock.json .
COPY spx-gui/package.json spx-gui/pnpm-lock.yaml .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The pnpm-workspace.yaml file contains critical configurations such as dependency overrides and allowBuilds. Since it is not copied into the Docker image before running pnpm install --frozen-lockfile, the installation will fail because the lockfile resolution (which depends on these overrides) will be out of sync. Copying pnpm-workspace.yaml along with package.json and pnpm-lock.yaml resolves this issue while maintaining Docker layer caching.

COPY spx-gui/package.json spx-gui/pnpm-lock.yaml spx-gui/pnpm-workspace.yaml .

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Handled in df13c89: Docker build now copies pnpm-workspace.yaml before frozen install.

Comment thread spx-gui/build/Dockerfile.account Outdated
WORKDIR /app/spx-gui

COPY spx-gui/package.json spx-gui/package-lock.json .
COPY spx-gui/package.json spx-gui/pnpm-lock.yaml .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The pnpm-workspace.yaml file contains critical configurations such as dependency overrides and allowBuilds. Since it is not copied into the Docker image before running pnpm install --frozen-lockfile, the installation will fail because the lockfile resolution (which depends on these overrides) will be out of sync. Copying pnpm-workspace.yaml along with package.json and pnpm-lock.yaml resolves this issue while maintaining Docker layer caching.

COPY spx-gui/package.json spx-gui/pnpm-lock.yaml spx-gui/pnpm-workspace.yaml .

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Handled in df13c89: Account Docker build now copies pnpm-workspace.yaml before frozen install.

Comment thread spx-gui/install-spx.sh Outdated
Comment on lines +22 to +27
acquire_lock() {
while ! mkdir "${SPX_LOCK_DIR}" 2>/dev/null; do
sleep 0.1
done
LOCK_ACQUIRED=true
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The current lock implementation will loop indefinitely if a stale lock directory is left behind (e.g., if a previous run was interrupted or killed with SIGKILL). This can cause local development or CI/CD pipelines to hang forever without any output. Adding a timeout (e.g., 60 seconds) with a clear error message prevents infinite hangs and helps with troubleshooting.

Suggested change
acquire_lock() {
while ! mkdir "${SPX_LOCK_DIR}" 2>/dev/null; do
sleep 0.1
done
LOCK_ACQUIRED=true
}
acquire_lock() {
local count=0
local max_attempts=600 # 60 seconds total
while ! mkdir "${SPX_LOCK_DIR}" 2>/dev/null; do
if (( count >= max_attempts )); then
echo "Failed to acquire lock at ${SPX_LOCK_DIR} after 60 seconds. If no other instance is running, please remove the lock directory manually." >&2
exit 1
fi
sleep 0.1
(( count++ ))
done
LOCK_ACQUIRED=true
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Handled in df13c89: lock acquisition now times out after 60 seconds with a cleanup hint.

@nighca nighca changed the title Migrate frontend installs to pnpm Reduce duplicate dependency storage across worktrees Jul 3, 2026
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.

Reduce duplicate dependency installation across local worktrees

1 participant