PaperClipAI

Jannik/PaperClipAI

0 Fork 0

mirror of https://github.com/paperclipai/paperclip.git synced 2026-10-09 06:15:21 +02:00

Commit Graph

Author	SHA1	Message	Date
Devin Foley	ffd62a4cbb	fix(adapter-utils): carry the workspace origin remote into transported git workspaces (#10873 ) ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - When an agent runs on a different host (sandbox or SSH), the adapter transport copies the local git execution workspace to that host and syncs changes back after the run > - The transport materializes the remote copy with `git init` plus a depth-1 or bundle fetch, so the copy has no `origin` remote and its head reads as a parentless snapshot commit > - An agent asked to publish its branch (push it, open a pull request) sees "no remote, root snapshot" and must hand the publish step back to a human operator, even when the branch base is a commit the upstream remote already holds > - This pull request carries the workspace's `origin` URL (credential-scrubbed) onto the transported copy as metadata > - The benefit is that branches produced in transported workspaces stay publishable by any actor with credentials, while the transport itself still never fetches or pushes ## Linked Issues or Issue Description No public issue exists. Description follows the enhancement template: What existing behavior does this improve? The workspace transport in `@paperclipai/adapter-utils` already copies a git workspace to the execution host and back. This change improves the fidelity of that copy: the transported repo keeps the workspace's `origin` remote instead of losing it. Subsystem affected Adapter utilities — the sandbox transport (`withShallowGitWorkspaceClone` in `packages/adapter-utils/src/git-workspace-sync.ts`) and the SSH transport (`importGitWorkspaceToSsh` in `packages/adapter-utils/src/ssh.ts`). Current behavior The transported copy is built with `git init` plus a depth-1 (sandbox) or bundle (SSH) fetch. It has no remotes. `git remote -v` is empty and the head commit reads as a root snapshot with no visible ancestry. Agents and operators inside the execution host cannot fetch real ancestry or push a branch, even when the branch base is a commit the upstream remote already holds. Proposed behavior The transport reads the source workspace's `origin` URL, scrubs credentials from it, and configures it on the transported copy. The sandbox path adds the remote to the fresh clone. The SSH path sets or adds the remote in the remote setup script, which also covers reused workspace directories. A workspace with no `origin` transports exactly as before. Reason and benefit A branch committed in a transported workspace becomes publishable in place: the shallow boundary commit already exists on the remote, so a push pack closes without full local ancestry (a new test locks in this property). Fetching real ancestry also becomes possible for whoever holds credentials. Without this, agents must describe their change in a handoff document and a human must reconstruct the branch by hand. Breaking changes None. The URL copy is best-effort and metadata-only. The transport never fetches from or pushes to the remote. The no-remote-git contract holds: sync-back through the local cwd stays the only cross-run persistence path, and `packages/adapters/AUTHORING.md` gains a paragraph that makes the carried-remote nuance explicit. ## What Changed - `packages/adapter-utils/src/git-workspace-sync.ts`: new `sanitizeGitRemoteUrl` (strips http(s) userinfo, where tokens can be embedded; scp-like/ssh forms and filesystem paths pass through) and `readSanitizedOriginRemoteUrl`; `withShallowGitWorkspaceClone` configures the scrubbed `origin` on the fresh clone, best-effort. - `packages/adapter-utils/src/ssh.ts`: `importGitWorkspaceToSsh` sets or adds the scrubbed `origin` in the remote setup script, non-fatal under `set -e`. - `packages/adapter-utils/src/git-workspace-sync.test.ts`: four new integration cases (remote copied, credentials scrubbed, no-origin unchanged, push from the shallow clone to an origin that holds the base commit) plus `sanitizeGitRemoteUrl` unit tests. - `packages/adapters/AUTHORING.md`: documents that a transported copy may carry a credential-scrubbed `origin` as metadata, and why this does not weaken the no-remote-git contract. ## Verification - `npx vitest run packages/adapter-utils/src/git-workspace-sync.test.ts` — 12/12 pass (4 new integration cases + sanitizer unit tests). - `npx vitest run packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 24/24 pass. - `npx vitest run packages/adapter-utils/src/ssh-fixture.test.ts` — 16/16 pass, including the `no-remote-git contract` case (a workspace without `origin` still round-trips with no remote introduced at any point). - `node scripts/check-no-git-push.mjs` — passes; this change adds no push or fetch to adapter/runtime code. - `pnpm typecheck` in `packages/adapter-utils` — clean. ## Risks - Low risk. The change is additive metadata on the transported copy only; failure to record the remote never fails the transport. - Credential exposure is the real hazard and is handled: http(s) userinfo is stripped before the URL leaves the host. Non-http forms (scp-like, `ssh://`) carry no secret in the URL and pass through. - A reused SSH workspace whose project `origin` changed now gets the current URL via `set-url` instead of keeping a stale one. ## Model Used Claude Fable 5 (`claude-fable-5`), Anthropic — extended thinking, agentic tool use via Claude Code CLI. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge	2026-08-05 10:24:01 -07:00
Devin FoleyandPaperclip	1f70fd9a22	PAPA-430: workspace finalize gates + no-remote-git enforcement (#6969 ) ## Thinking Path > - Paperclip orchestrates AI agents across isolated execution workspaces; the local cwd is the only persistence boundary between runs. > - Workspace lifecycle (worktree_prepare → execute → workspace_finalize) and the wake/accept flow are what guarantee that dependent issues see a consistent worktree. > - PAPA-380 / PAPA-431 / PAPA-432 / PAPA-440 surfaced three holes in that contract: silent env reuse across assignees, dependent wakes firing before finalize, and `issue.interaction.accept` advancing before finalize landed. > - PAPA-441 / PAPA-442 then needed to document the "no remote git" contract and prevent future adapter/runtime code from quietly reintroducing `git push` as a backdoor sync. > - This pull request lands those server fixes, the static `check-no-git-push` enforcement, the AUTHORING.md cross-link, and the Cody-review follow-ups on the PAPA-430 thread. > - The benefit is that finalize is a real barrier — board accepts, dependent wakes, and operator-set env all respect it — and adapter code can't bypass it via raw `git push`. ## What Changed - server (PAPA-380, PAPA-431): `execution-workspace-policy` refuses silent env reuse when the assignee's resolved env disagrees with the workspace it would inherit. The inheritance protection is now scoped to the actual inheritance signal — explicit issue-level `environmentId` is honored even when the agent's default env is `null`. - server (PAPA-432): `heartbeat.ts` gates dependent wakes on `listUnfinalizedExecutionWorkspaceIds`, and writes a `workspace_finalize` row on the succeeded path. Write failures now surface instead of being swallowed so dependents aren't silently stranded behind a missing row. - server (PAPA-440): `issue-thread-interactions.acceptInteraction` adds a workspace_finalize precondition for `request_confirmation` (not `suggest_tasks`). Accept returns 409 if finalize hasn't succeeded for the latest workspace operation. - ci (PAPA-442): new `scripts/check-no-git-push.mjs` static check scans `packages/adapters/`, `packages/adapter-utils/`, `server/src/`, and `cli/src/` for any `git push` invocation (string or args-array). Wired into the `policy` PR job and `test:release-registry`. Operators can opt in per-call with `// paperclip:allow-git-push: <reason>`. Release scripts are out of scope by design. - docs (PAPA-441): `AUTHORING.md` documents the no-remote-git contract and cross-links the static check so adapter authors learn the rule and the enforcement together. - review follow-up (PAPA-430, Cody): three fixes — env resolver bug, accept-gate scope (request_confirmation only), and finalize record write on the succeeded path. ## Verification - `pnpm exec vitest run server/src/__tests__/execution-workspace-policy.test.ts server/src/__tests__/issue-thread-interactions-service.test.ts` → 33/33 pass - `node scripts/check-no-git-push.test.mjs` → check covers string form, args-array form, comment exclusions, and per-line allow-comment. - Manual: server compiles; the policy job runs the check in <1s before heavier jobs. ## Risks - Behavioral shift in accept: boards accepting `request_confirmation` while finalize is in-flight now get 409s. This is intentional — they can retry — but it changes timing on a hot path. `suggest_tasks` is unaffected. - Workspace policy: the env-reuse refusal is a new error path. Issues that previously silently reused an env from a different-assignee workspace will now fail-loud; the resolver still honors explicit issue-level `executionWorkspaceSettings.environmentId`. - CI rule: any future legitimate `git push` in scoped dirs must be marked with the allow-comment, which is the intended ergonomic. ## Model Used - Claude Opus 4.7 (`claude-opus-4-7`, extended thinking), via Claude Code in the Paperclip executor adapter. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — server/CI/docs only) - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge Closes related issues: PAPA-430, PAPA-380, PAPA-431, PAPA-432, PAPA-440, PAPA-441, PAPA-442 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>	2026-05-29 08:25:29 -07:00

Author

SHA1

Message

Date

Devin Foley

ffd62a4cbb

fix(adapter-utils): carry the workspace origin remote into transported git workspaces (#10873 )

## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - When an agent runs on a different host (sandbox or SSH), the adapter
transport copies the local git execution workspace to that host and
syncs changes back after the run
> - The transport materializes the remote copy with `git init` plus a
depth-1 or bundle fetch, so the copy has no `origin` remote and its head
reads as a parentless snapshot commit
> - An agent asked to publish its branch (push it, open a pull request)
sees "no remote, root snapshot" and must hand the publish step back to a
human operator, even when the branch base is a commit the upstream
remote already holds
> - This pull request carries the workspace's `origin` URL
(credential-scrubbed) onto the transported copy as metadata
> - The benefit is that branches produced in transported workspaces stay
publishable by any actor with credentials, while the transport itself
still never fetches or pushes

## Linked Issues or Issue Description

No public issue exists. Description follows the enhancement template:

**What existing behavior does this improve?**

The workspace transport in `@paperclipai/adapter-utils` already copies a
git workspace to the execution host and back. This change improves the
fidelity of that copy: the transported repo keeps the workspace's
`origin` remote instead of losing it.

**Subsystem affected**

Adapter utilities — the sandbox transport
(`withShallowGitWorkspaceClone` in
`packages/adapter-utils/src/git-workspace-sync.ts`) and the SSH
transport (`importGitWorkspaceToSsh` in
`packages/adapter-utils/src/ssh.ts`).

**Current behavior**

The transported copy is built with `git init` plus a depth-1 (sandbox)
or bundle (SSH) fetch. It has no remotes. `git remote -v` is empty and
the head commit reads as a root snapshot with no visible ancestry.
Agents and operators inside the execution host cannot fetch real
ancestry or push a branch, even when the branch base is a commit the
upstream remote already holds.

**Proposed behavior**

The transport reads the source workspace's `origin` URL, scrubs
credentials from it, and configures it on the transported copy. The
sandbox path adds the remote to the fresh clone. The SSH path sets or
adds the remote in the remote setup script, which also covers reused
workspace directories. A workspace with no `origin` transports exactly
as before.

**Reason and benefit**

A branch committed in a transported workspace becomes publishable in
place: the shallow boundary commit already exists on the remote, so a
push pack closes without full local ancestry (a new test locks in this
property). Fetching real ancestry also becomes possible for whoever
holds credentials. Without this, agents must describe their change in a
handoff document and a human must reconstruct the branch by hand.

**Breaking changes**

None. The URL copy is best-effort and metadata-only. The transport never
fetches from or pushes to the remote. The no-remote-git contract holds:
sync-back through the local cwd stays the only cross-run persistence
path, and `packages/adapters/AUTHORING.md` gains a paragraph that makes
the carried-remote nuance explicit.

## What Changed

- `packages/adapter-utils/src/git-workspace-sync.ts`: new
`sanitizeGitRemoteUrl` (strips http(s) userinfo, where tokens can be
embedded; scp-like/ssh forms and filesystem paths pass through) and
`readSanitizedOriginRemoteUrl`; `withShallowGitWorkspaceClone`
configures the scrubbed `origin` on the fresh clone, best-effort.
- `packages/adapter-utils/src/ssh.ts`: `importGitWorkspaceToSsh` sets or
adds the scrubbed `origin` in the remote setup script, non-fatal under
`set -e`.
- `packages/adapter-utils/src/git-workspace-sync.test.ts`: four new
integration cases (remote copied, credentials scrubbed, no-origin
unchanged, push from the shallow clone to an origin that holds the base
commit) plus `sanitizeGitRemoteUrl` unit tests.
- `packages/adapters/AUTHORING.md`: documents that a transported copy
may carry a credential-scrubbed `origin` as metadata, and why this does
not weaken the no-remote-git contract.

## Verification

- `npx vitest run packages/adapter-utils/src/git-workspace-sync.test.ts`
— 12/12 pass (4 new integration cases + sanitizer unit tests).
- `npx vitest run
packages/adapter-utils/src/sandbox-managed-runtime.test.ts` — 24/24
pass.
- `npx vitest run packages/adapter-utils/src/ssh-fixture.test.ts` —
16/16 pass, including the `no-remote-git contract` case (a workspace
without `origin` still round-trips with no remote introduced at any
point).
- `node scripts/check-no-git-push.mjs` — passes; this change adds no
push or fetch to adapter/runtime code.
- `pnpm typecheck` in `packages/adapter-utils` — clean.

## Risks

- Low risk. The change is additive metadata on the transported copy
only; failure to record the remote never fails the transport.
- Credential exposure is the real hazard and is handled: http(s)
userinfo is stripped before the URL leaves the host. Non-http forms
(scp-like, `ssh://`) carry no secret in the URL and pass through.
- A reused SSH workspace whose project `origin` changed now gets the
current URL via `set-url` instead of keeping a stale one.

## Model Used

Claude Fable 5 (`claude-fable-5`), Anthropic — extended thinking,
agentic tool use via Claude Code CLI.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change (e.g. `docs/...`, `fix/...`)
and contains no internal Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] All Paperclip CI gates are green
- [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [x] I will address all Greptile and reviewer comments before
requesting merge

2026-08-05 10:24:01 -07:00

Devin FoleyandPaperclip

1f70fd9a22

PAPA-430: workspace finalize gates + no-remote-git enforcement (#6969 )

## Thinking Path

> - Paperclip orchestrates AI agents across isolated execution
workspaces; the local cwd is the only persistence boundary between runs.
> - Workspace lifecycle (worktree_prepare → execute →
workspace_finalize) and the wake/accept flow are what guarantee that
dependent issues see a consistent worktree.
> - PAPA-380 / PAPA-431 / PAPA-432 / PAPA-440 surfaced three holes in
that contract: silent env reuse across assignees, dependent wakes firing
before finalize, and `issue.interaction.accept` advancing before
finalize landed.
> - PAPA-441 / PAPA-442 then needed to document the "no remote git"
contract and prevent future adapter/runtime code from quietly
reintroducing `git push` as a backdoor sync.
> - This pull request lands those server fixes, the static
`check-no-git-push` enforcement, the AUTHORING.md cross-link, and the
Cody-review follow-ups on the PAPA-430 thread.
> - The benefit is that finalize is a real barrier — board accepts,
dependent wakes, and operator-set env all respect it — and adapter code
can't bypass it via raw `git push`.

## What Changed

- **server (PAPA-380, PAPA-431):** `execution-workspace-policy` refuses
silent env reuse when the assignee's resolved env disagrees with the
workspace it would inherit. The inheritance protection is now scoped to
the actual inheritance signal — explicit issue-level `environmentId` is
honored even when the agent's default env is `null`.
- **server (PAPA-432):** `heartbeat.ts` gates dependent wakes on
`listUnfinalizedExecutionWorkspaceIds`, and writes a
`workspace_finalize` row on the succeeded path. Write failures now
surface instead of being swallowed so dependents aren't silently
stranded behind a missing row.
- **server (PAPA-440):** `issue-thread-interactions.acceptInteraction`
adds a workspace_finalize precondition for `request_confirmation` (not
`suggest_tasks`). Accept returns 409 if finalize hasn't succeeded for
the latest workspace operation.
- **ci (PAPA-442):** new `scripts/check-no-git-push.mjs` static check
scans `packages/adapters/`, `packages/adapter-utils/`, `server/src/`,
and `cli/src/` for any `git push` invocation (string or args-array).
Wired into the `policy` PR job and `test:release-registry`. Operators
can opt in per-call with `// paperclip:allow-git-push: <reason>`.
Release scripts are out of scope by design.
- **docs (PAPA-441):** `AUTHORING.md` documents the no-remote-git
contract and cross-links the static check so adapter authors learn the
rule and the enforcement together.
- **review follow-up (PAPA-430, Cody):** three fixes — env resolver bug,
accept-gate scope (request_confirmation only), and finalize record write
on the succeeded path.

## Verification

- `pnpm exec vitest run
server/src/__tests__/execution-workspace-policy.test.ts
server/src/__tests__/issue-thread-interactions-service.test.ts` → 33/33
pass
- `node scripts/check-no-git-push.test.mjs` → check covers string form,
args-array form, comment exclusions, and per-line allow-comment.
- Manual: server compiles; the policy job runs the check in <1s before
heavier jobs.

## Risks

- **Behavioral shift in accept:** boards accepting
`request_confirmation` while finalize is in-flight now get 409s. This is
intentional — they can retry — but it changes timing on a hot path.
`suggest_tasks` is unaffected.
- **Workspace policy:** the env-reuse refusal is a new error path.
Issues that previously silently reused an env from a different-assignee
workspace will now fail-loud; the resolver still honors explicit
issue-level `executionWorkspaceSettings.environmentId`.
- **CI rule:** any future legitimate `git push` in scoped dirs must be
marked with the allow-comment, which is the intended ergonomic.

## Model Used

- Claude Opus 4.7 (`claude-opus-4-7`, extended thinking), via Claude
Code in the Paperclip executor adapter.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have run tests locally and they pass
- [x] I have added or updated tests where applicable
- [ ] If this change affects the UI, I have included before/after
screenshots (N/A — server/CI/docs only)
- [x] I have updated relevant documentation to reflect my changes
- [x] I have considered and documented any risks above
- [x] I will address all Greptile and reviewer comments before
requesting merge

Closes related issues: PAPA-430, PAPA-380, PAPA-431, PAPA-432, PAPA-440,
PAPA-441, PAPA-442

---------

Co-authored-by: Paperclip <noreply@paperclip.ing>

2026-05-29 08:25:29 -07:00