mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
DottaandPaperclip
159d006bb4
## Thinking Path > - Paperclip manages AI agents and their connected accounts. > - Login flows bind verified credentials to an owned connection. > - The shared save service currently uses the provider name to choose a session ID column and completion state. > - A provider can support more than one login flow. Provider identity does not define session identity or state. > - This PR gives every flow the same internal session ID contract and lets the setup-token owner complete its own claim. > - Credential writes, session binding, and completion remain in one transaction. ## Linked Issues or Issue Description Refs #17106. This is an independent prerequisite for that sleep cleanup PR. It contains no scheduler or cleanup-worker changes. **Current behavior** The connection save service looks up Anthropic sessions by public ID and all other sessions by internal ID. It also marks every Anthropic session as stored. Local Anthropic login works because its public and internal IDs happen to match, and its caller overwrites that intermediate state. **Proposed behavior** Every caller passes an internal session ID. A small setup-token completion service resolves the public ID within its owner scope and supplies a transaction callback. That callback uses the existing conditional stored transition. Device and local login flows keep their own final states. **Reason and benefit** Adding or changing a login flow no longer requires provider branches in the shared session binding path. The service still owns membership checks, row locking, target validation, credential writes, and idempotent binding. The flow owns only public-ID resolution and its completion transition. **Breaking changes** The internal TypeScript save signature changes. All production callers are updated. Public HTTP payloads, database schema, and local session URLs stay unchanged. ## What Changed - Replace the ambiguous session ID argument with an internal session reference and an optional completion callback. - Run the callback inside the existing save transaction, only for the first successful binding. - Move public-ID resolution and the stored transition to setup-token-ai-connection.ts. - Update setup-token, device, and local callers. - Add real PostgreSQL tests for ID selection, owner and target checks, one-time completion, and rollback after a rejected completion. - Exercise local Anthropic and OpenAI HTTP completion with distinct public and internal IDs. - Document the ownership contract in AI-CONNECTIONS.md. ## Verification - Passed: pnpm -r typecheck. - Passed: pnpm exec vitest run server/src/__tests__/ai-connections.test.ts server/src/__tests__/agent-device-login-routes.test.ts server/src/__tests__/agents-claude-oauth-binding.test.ts — 233 tests. - Passed: pnpm build. - Passed: full CI, including test shards, typecheck, build, and E2E, ond98ce6e504: https://github.com/paperclipai/paperclip/actions/runs/39654210168. - The additional local pnpm test:run was stopped after the full CI pass to avoid duplicating the completed run. It is not reported as a completed local full-suite pass. - Greptile: 5/5 ond98ce6e504, with no actionable findings or unresolved threads. - The first AI suite run had two new fixture failures: a duplicate environment name across provider cases, and an assertion that ignored existing shared connections. The fixtures now use distinct names and query the attempted connection directly. The corrected affected suites passed. The first CI run passed all 1,420 assertions in server shard 11/12 but failed on an unhandled child-process rejection in `workspace-runtime-exposure.test.ts` (the synthetic EADDRINUSE case). Both the test and runtime implementation are unchanged from this PR's base. The two affected parameter cases passed locally unchanged (2 passed, 29 skipped). Inspection shows a possible timing window before the early-exit promise joins the readiness race; that cause has not been reproduced conclusively. The single CI retry passed on the same commit. This demonstrates recovery; the original timing cause was not conclusively reproduced. Original job: https://github.com/paperclipai/paperclip/actions/runs/39654210168/job/119410225203. ## Risks - A completion callback failure must roll back the credential and session binding. The real-database regression test proves this for a rejected setup-token claim. - The callback is internal, awaited inside the existing transaction, and must use the supplied transaction handle. It adds no network work or provider SDK dependency. - Setup-token resolution adds one indexed lookup per completion. The shared save then locks and validates the row again. No recurring work is added. - The setup-token owner now uses the existing conditional completion guard, including the unconsumed-claim and commit-time expiry checks. Invalid claims fail closed. - Live provider sign-in was not exercised; tests use fixture credentials and real PostgreSQL. No schema migration is needed. ## Model Used OpenAI GPT-6 through Codex, with reasoning, source inspection, code editing, and test execution. The exact serving model ID and context-window size are not exposed in this session. ## 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 Co-authored-by: Paperclip <noreply@paperclip.ing>