mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
DottaandPaperclip
cde944ed75
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Before a run starts, repository admission checks every retained
reader of the physical repository root and of overlapping roots.
> - Each retained reader records the run, task, agent and responsible
user that the server admitted.
> - A wake for another task can coalesce into a running run. The merge
replaces `contextSnapshot.issueId` and `taskId` with the other task.
> - The retained-reader check used that mutable context task before the
run's immutable task binding. The old reader then looked like a changed
identity.
> - When a later run admits an overlapping root, such as a shared
project checkout that contains earlier task worktrees, every admission
on that root fails. The root stays blocked and no operator action can
repair it.
> - This pull request accepts a retained reader when its task matches
the run's immutable binding or its current context task.
> - The benefit is that legitimate wake coalescing no longer blocks a
shared root, and missing or truly mismatched identities still fail
closed.
## Linked Issues or Issue Description
Refs #16717 and #16832. Those cover retained readers whose run, agent or
task was deleted. This pull request covers a different cause: every
identity still exists, but wake coalescing changed the run's context
task. #16832 keeps identity mismatches fail-closed, so it does not fix
this case. Both pull requests change the same condition in
`execution-workspace-root-readers.ts`. The pull request that merges
second must rebase.
**What happened?**
A scheduled routine run failed in setup with `Repository root retains a
reader whose identity is no longer available` (403). Every later run of
the routine failed in the same way, for eight hours in a row. Two
earlier runs of the same agent were the retained readers. Each run
started for its own task and recorded its reader. About 10 to 16 minutes
later, a monitor wake for a different task coalesced into the running
run with reason `issue_execution_same_name`.
`mergeCoalescedContextSnapshot` set `contextSnapshot.issueId` to the
other task. `heartbeat_runs.issue_id` stayed on the original task,
because the `heartbeat_runs_set_scope_kind` trigger makes the binding
immutable. The readers were on per-task worktree roots, so nothing
failed at first. Then the routine's tasks changed to the shared project
checkout. That root contains those worktrees, so admission loaded the
two readers, compared them with the changed context task, and threw.
**Expected behavior**
A retained reader stays valid while its run, agent, task and responsible
user still exist and still match what the server admitted. A context
change from wake coalescing must not make the reader unavailable. A
reader whose task matches neither the run's binding nor its context must
still be rejected.
**Steps to reproduce**
1. Admit run R for task A on a nested root, for example
`<checkout>/.paperclip/worktrees/a`.
2. Coalesce a wake for task B into R. `contextSnapshot.issueId` becomes
B. `heartbeat_runs.issue_id` stays A.
3. Admit or publish another task on the parent root `<checkout>`.
4. Observe the 403 `Repository root retains a reader whose identity is
no longer available`. Every later admission on that root fails in the
same way.
**Paperclip version or commit**
Reproduced on `master` at b44871098 with the new regression test.
## What Changed
- Add `runRetainsIssueIdentity(run, issueId)` in
`server/src/services/execution-workspace-root-readers.ts`. It accepts
the previous derived identity (`nativeIssueId ?? context issueId ??
context taskId ?? issueId`) or the immutable binding (`nativeIssueId ??
issueId`).
- Use the helper for the three retained-run identity checks:
`loadRepositoryRootReaders`, `requiresAnonymousRepositoryAcquisition`
and `assertRootProjectTrust`.
- Add a real PostgreSQL regression. A reader on a nested worktree root
is admitted for task A. The test builds the coalesced context with the
real `mergeCoalescedContextSnapshot`. Publication on the parent root
must succeed. A reader whose task matches neither the binding nor the
context must still fail.
- Move one existing row. The old "rejects" test changed only the context
task, which is now the legitimate coalescing case. The test now changes
the reader's recorded task to a different existing task, so the mismatch
path still fails closed.
- Update the retained-reader contract in `doc/DATABASE.md`.
## Verification
- Red before the fix: `pnpm exec vitest run --project
@paperclipai/server --maxWorkers=1 --fileParallelism=false
server/src/__tests__/repository-root-reader-queries.test.ts` fails 1 of
9 tests. The new test passes its first publication. It fails after the
coalesced context with `Repository root retains a reader whose identity
is no longer available` (403), which is the production error.
- Green after the fix: the same command passes 9 of 9 tests.
- One intermediate attempt failed and is kept here. My first test draft
changed `heartbeat_runs.issue_id`. The database trigger rejected it with
`run task binding is immutable`. That rejection confirms the binding
this fix relies on. The final test uses a separate run for the negative
case.
- Related suites: `task-workspace-repositories*.test.ts`,
`task-workspace-source-copy-privacy.test.ts`,
`project-workspace-root-provenance.test.ts`,
`workspace-authorization-query.test.ts`,
`execution-transaction-timing.test.ts`,
`execution-workspace-reopen.test.ts`, `file-resources.test.ts`,
`heartbeat-project-repositories.test.ts`,
`heartbeat-workspace-source-trust.test.ts` and
`heartbeat/workspace-preparation.test.ts`. All 13 files pass on the
fixed head. The first invocation passed 8 files. The other 5 files
failed to import `@paperclipai/plugin-sdk`, because its `dist` was not
built in this fresh checkout. That run reported 37 failed tests, all
from the import error, not from this change. After `pnpm --filter
@paperclipai/plugin-sdk... build`, the same 5 files passed 206 of 206
tests.
- Production evidence (read-only SQL on the affected instance): exactly
two readers in the company fail the current check. Both are on the
blocked root. For both, `heartbeat_runs.issue_id` equals the reader's
task and the context task is a different task after a coalesced
`issue_execution_same_name` wake. Both pass the new rule, so the blocked
root recovers after deployment without data repair.
- Not run: full `pnpm -r typecheck`, `pnpm test:run` and `pnpm build`.
The sandbox has 4 GB of memory. CI runs these checks.
## Risks
- The rule now also accepts a reader whose task equals the run's
immutable binding while the context points to another task. The binding
is the run's privacy binding. The database sets it at insert and does
not let it change, and it requires `native_issue_id` to agree with it.
The reader's own recorded task is still the authorization context.
- The rule keeps the previous context identity. A run that coalesced a
wake before it started and then coalesced another one later could still
produce an unavailable reader. I did not find that sequence in the
affected data. The cross-task coalesce targets a run that already holds
the other task's execution lock, and that happens after admission.
- No schema, migration or API change.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`) through Claude Code in a Paperclip
agent run, with tool use, code execution and read-only SQL diagnostics.
The context window and reasoning settings are managed by the runtime.
## 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
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Paperclip <noreply@paperclip.ing>