Skip to content

Add package manager support to init scaffolding#226

Open
xstelea wants to merge 1 commit intomattpocock:mainfrom
xstelea:main
Open

Add package manager support to init scaffolding#226
xstelea wants to merge 1 commit intomattpocock:mainfrom
xstelea:main

Conversation

@xstelea
Copy link
Copy Markdown

@xstelea xstelea commented Apr 3, 2026

Allow users to select npm, pnpm, yarn, or bun during sandcastle init via a new --package-manager flag or interactive prompt. Dockerfiles are patched with corepack/bun install lines, and all generated .ts/.md files have npm references rewritten to the chosen package manager.

Allow users to select npm, pnpm, yarn, or bun during `sandcastle init`
via a new `--package-manager` flag or interactive prompt. Dockerfiles are
patched with corepack/bun install lines, and all generated .ts/.md files
have npm references rewritten to the chosen package manager.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread src/InitService.test.ts
const lines = getNextStepsLines("simple-loop");
const joined = lines.join("\n");
expect(joined).toContain("npm install");
expect(joined).toContain("onSandboxReady");
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Why was this removed?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Accidental removal, will restore it.

@mattpocock
Copy link
Copy Markdown
Owner

This is a good idea in theory. I'm just not sure whether I want it for right now.

Since there will be probably a lot of churn in the repo, I'm wondering whether I want this relatively inessential feature now or whether I want to keep this PR for reference and implement it later.

Was this something that felt like a friction point in the setup for you?

@xstelea
Copy link
Copy Markdown
Author

xstelea commented Apr 5, 2026

This is a good idea in theory. I'm just not sure whether I want it for right now.

Since there will be probably a lot of churn in the repo, I'm wondering whether I want this relatively inessential feature now or whether I want to keep this PR for reference and implement it later.

Was this something that felt like a friction point in the setup for you?

Makes sense. Feel free to close the PR

My reason for this change was to enable feedback loops through pre-commit hooks. There is probably an alternative way to solve this by directly editing the main.ts file.

@joao-tail
Copy link
Copy Markdown

joao-tail commented Apr 5, 2026

Hey @mattpocock!, wanted to share something we hit today that's directly related to this PR.

We're following the same plan → implement + review → merge orchestration you use in this repo, adapted for our pnpm monorepo. Since the default Dockerfile only has npm, our agents ran into trouble.

During one run, the Implementer agent couldn't find pnpm, tried a few things, and landed on sudo npm install -g pnpm. It worked — agent finished, tests passed, committed, closed the issue. But the sudo changed file ownership under /home/agent/. When the Reviewer started on the same container, withSandboxLifecycle ran git config --global user.name "RALPH" and got exit 128 — .gitconfig was now root-owned.

The Reviewer crash caused Promise.allSettled() to reject the whole issue. The Merger skipped it. We ended up with a fully implemented package that never landed on main, while the GitHub issue showed as closed.

Hard to diagnose because the exit 128 doesn't hint at ownership, the root cause happened in a different agent phase, and 14/15 parallel issues succeeded so the run looked healthy.

Fix was simple — added this before USER agent:

RUN corepack enable && corepack prepare pnpm@9.15.4 --activate

I think the Dockerfile generation part of this PR would prevent this class of failure out of the box. Even if the full init rewrite isn't a priority, having the right corepack/bun lines in the generated Dockerfile addresses a correctness issue, not just convenience.

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.

3 participants