mirror of
https://github.com/paperclipai/paperclip
synced 2026-04-25 17:25:15 +02:00
Compare commits
6 Commits
pap-1078-q
...
PAPA-45-up
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
347f38019f | ||
|
|
25615407a4 | ||
|
|
f843a45a84 | ||
|
|
36049beeea | ||
|
|
c041fee6fc | ||
|
|
82290451d4 |
16
.github/PULL_REQUEST_TEMPLATE.md
vendored
16
.github/PULL_REQUEST_TEMPLATE.md
vendored
@@ -38,9 +38,25 @@
|
||||
|
||||
-
|
||||
|
||||
## Model Used
|
||||
|
||||
<!--
|
||||
Required. Specify which AI model was used to produce or assist with
|
||||
this change. Be as descriptive as possible — include:
|
||||
• Provider and model name (e.g., Claude, GPT, Gemini, Codex)
|
||||
• Exact model ID or version (e.g., claude-opus-4-6, gpt-4-turbo-2024-04-09)
|
||||
• Context window size if relevant (e.g., 1M context)
|
||||
• Reasoning/thinking mode if applicable (e.g., extended thinking, chain-of-thought)
|
||||
• Any other relevant capability details (e.g., tool use, code execution)
|
||||
If no AI model was used, write "None — human-authored".
|
||||
-->
|
||||
|
||||
-
|
||||
|
||||
## Checklist
|
||||
|
||||
- [ ] I have included a thinking path that traces from project context to this change
|
||||
- [ ] I have specified the model used (with version and capability details)
|
||||
- [ ] I have run tests locally and they pass
|
||||
- [ ] I have added or updated tests where applicable
|
||||
- [ ] If this change affects the UI, I have included before/after screenshots
|
||||
|
||||
@@ -11,8 +11,9 @@ We really appreciate both small fixes and thoughtful larger changes.
|
||||
- Pick **one** clear thing to fix/improve
|
||||
- Touch the **smallest possible number of files**
|
||||
- Make sure the change is very targeted and easy to review
|
||||
- All automated checks pass (including Greptile comments)
|
||||
- No new lint/test failures
|
||||
- All tests pass and CI is green
|
||||
- Greptile score is 5/5 with all comments addressed
|
||||
- Use the [PR template](.github/PULL_REQUEST_TEMPLATE.md)
|
||||
|
||||
These almost always get merged quickly when they're clean.
|
||||
|
||||
@@ -26,11 +27,26 @@ These almost always get merged quickly when they're clean.
|
||||
- Before / After screenshots (or short video if UI/behavior change)
|
||||
- Clear description of what & why
|
||||
- Proof it works (manual testing notes)
|
||||
- All tests passing
|
||||
- All Greptile + other PR comments addressed
|
||||
- All tests passing and CI green
|
||||
- Greptile score 5/5 with all comments addressed
|
||||
- [PR template](.github/PULL_REQUEST_TEMPLATE.md) fully filled out
|
||||
|
||||
PRs that follow this path are **much** more likely to be accepted, even when they're large.
|
||||
|
||||
## PR Requirements (all PRs)
|
||||
|
||||
### Use the PR Template
|
||||
|
||||
Every pull request **must** follow the PR template at [`.github/PULL_REQUEST_TEMPLATE.md`](.github/PULL_REQUEST_TEMPLATE.md). If you create a PR via the GitHub API or other tooling that bypasses the template, copy its contents into your PR description manually. The template includes required sections: Thinking Path, What Changed, Verification, Risks, and a Checklist.
|
||||
|
||||
### Tests Must Pass
|
||||
|
||||
All tests must pass before a PR can be merged. Run them locally first and verify CI is green after pushing.
|
||||
|
||||
### Greptile Review
|
||||
|
||||
We use [Greptile](https://greptile.com) for automated code review. Your PR must achieve a **5/5 Greptile score** with **all Greptile comments addressed** before it can be merged. If Greptile leaves comments, fix or respond to each one and request a re-review.
|
||||
|
||||
## General Rules (both paths)
|
||||
|
||||
- Write clear commit messages
|
||||
@@ -41,7 +57,7 @@ PRs that follow this path are **much** more likely to be accepted, even when the
|
||||
|
||||
## Writing a Good PR message
|
||||
|
||||
Please include a "thinking path" at the top of your PR message that explains from the top of the project down to what you fixed. E.g.:
|
||||
Your PR description must follow the [PR template](.github/PULL_REQUEST_TEMPLATE.md). All sections are required. The "thinking path" at the top explains from the top of the project down to what you fixed. E.g.:
|
||||
|
||||
### Thinking Path Example 1:
|
||||
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
realizeExecutionWorkspace,
|
||||
releaseRuntimeServicesForRun,
|
||||
resetRuntimeServicesForTests,
|
||||
resolveShell,
|
||||
sanitizeRuntimeServiceBaseEnv,
|
||||
stopRuntimeServicesForExecutionWorkspace,
|
||||
type RealizedExecutionWorkspace,
|
||||
@@ -1345,6 +1346,60 @@ describe("ensureRuntimeServicesForRun", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveShell (shell fallback)", () => {
|
||||
const originalShell = process.env.SHELL;
|
||||
const originalPlatform = process.platform;
|
||||
|
||||
afterEach(() => {
|
||||
if (originalShell !== undefined) {
|
||||
process.env.SHELL = originalShell;
|
||||
} else {
|
||||
delete process.env.SHELL;
|
||||
}
|
||||
Object.defineProperty(process, "platform", { value: originalPlatform });
|
||||
});
|
||||
|
||||
it("returns process.env.SHELL when set", () => {
|
||||
process.env.SHELL = "/usr/bin/zsh";
|
||||
expect(resolveShell()).toBe("/usr/bin/zsh");
|
||||
});
|
||||
|
||||
it("trims whitespace from SHELL env var", () => {
|
||||
process.env.SHELL = " /usr/bin/fish ";
|
||||
expect(resolveShell()).toBe("/usr/bin/fish");
|
||||
});
|
||||
|
||||
it("falls back to /bin/sh on non-Windows when SHELL is unset", () => {
|
||||
delete process.env.SHELL;
|
||||
Object.defineProperty(process, "platform", { value: "linux" });
|
||||
expect(resolveShell()).toBe("/bin/sh");
|
||||
});
|
||||
|
||||
it("falls back to sh (bare) on Windows when SHELL is unset", () => {
|
||||
delete process.env.SHELL;
|
||||
Object.defineProperty(process, "platform", { value: "win32" });
|
||||
expect(resolveShell()).toBe("sh");
|
||||
});
|
||||
|
||||
it("falls back to /bin/sh on darwin when SHELL is unset", () => {
|
||||
delete process.env.SHELL;
|
||||
Object.defineProperty(process, "platform", { value: "darwin" });
|
||||
expect(resolveShell()).toBe("/bin/sh");
|
||||
});
|
||||
|
||||
it("treats empty SHELL as unset and uses platform fallback", () => {
|
||||
process.env.SHELL = "";
|
||||
Object.defineProperty(process, "platform", { value: "linux" });
|
||||
expect(resolveShell()).toBe("/bin/sh");
|
||||
});
|
||||
|
||||
it("treats whitespace-only SHELL as unset and uses platform fallback", () => {
|
||||
process.env.SHELL = " ";
|
||||
Object.defineProperty(process, "platform", { value: "win32" });
|
||||
expect(resolveShell()).toBe("sh");
|
||||
});
|
||||
});
|
||||
|
||||
describeEmbeddedPostgres("workspace runtime startup reconciliation", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
@@ -24,6 +24,10 @@ import type { WorkspaceOperationRecorder } from "./workspace-operations.js";
|
||||
import { readExecutionWorkspaceConfig } from "./execution-workspaces.js";
|
||||
import { readProjectWorkspaceRuntimeConfig } from "./project-workspace-runtime-config.js";
|
||||
|
||||
export function resolveShell(): string {
|
||||
return process.env.SHELL?.trim() || (process.platform === "win32" ? "sh" : "/bin/sh");
|
||||
}
|
||||
|
||||
export interface ExecutionWorkspaceInput {
|
||||
baseCwd: string;
|
||||
source: "project_primary" | "task_session" | "agent_home";
|
||||
@@ -379,7 +383,7 @@ async function runWorkspaceCommand(input: {
|
||||
env: NodeJS.ProcessEnv;
|
||||
label: string;
|
||||
}) {
|
||||
const shell = process.env.SHELL?.trim() || "/bin/sh";
|
||||
const shell = resolveShell();
|
||||
const proc = await executeProcess({
|
||||
command: shell,
|
||||
args: ["-c", input.command],
|
||||
@@ -475,7 +479,7 @@ async function recordWorkspaceCommandOperation(
|
||||
cwd: input.cwd,
|
||||
metadata: input.metadata ?? null,
|
||||
run: async () => {
|
||||
const shell = process.env.SHELL?.trim() || "/bin/sh";
|
||||
const shell = resolveShell();
|
||||
const result = await executeProcess({
|
||||
command: shell,
|
||||
args: ["-c", input.command],
|
||||
@@ -1285,6 +1289,7 @@ async function startLocalRuntimeService(input: {
|
||||
const portEnvKey = asString(portConfig.envKey, "PORT");
|
||||
env[portEnvKey] = String(port);
|
||||
}
|
||||
|
||||
const expose = parseObject(input.service.expose);
|
||||
const readiness = parseObject(input.service.readiness);
|
||||
const urlTemplate =
|
||||
@@ -1359,7 +1364,8 @@ async function startLocalRuntimeService(input: {
|
||||
);
|
||||
}
|
||||
}
|
||||
const shell = process.env.SHELL?.trim() || "/bin/sh";
|
||||
|
||||
const shell = resolveShell();
|
||||
const child = spawn(shell, ["-lc", command], {
|
||||
cwd: serviceCwd,
|
||||
env,
|
||||
|
||||
Reference in New Issue
Block a user