mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
23bd238f05
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - The Costs page shows one card for each AI subscription account. Each
card comes from an `ai_subscriptions` row.
> - Paperclip cannot verify an xAI/Grok account with the provider. So
each Grok row used a key made from a hash of the raw credential.
> - The Grok CLI refreshes its OAuth tokens during runs. Paperclip
writes the refreshed `auth.json` back to the vault and keeps the
secret's `aiSessionEpoch`.
> - The next run sees a new credential hash.
`subscriptionService.register()` finds no match and adds a new
subscription row. The old row stays on the Costs page. One Grok
connection becomes many duplicate cards.
> - Claude and OpenAI rows usually avoid this problem, because a
provider profile check merges them. Grok rows never get verified, so
nothing merges them.
> - This pull request keys unverified rows on the credential session
(secret id + `aiSessionEpoch`), not on the token bytes.
> - The benefit is one subscription card per Grok connection through
token refreshes. A real reconnect still gets its own card.
## Linked Issues or Issue Description
No public issue exists. Bug description:
**What happened?**
On a hosted staging instance, the Costs → Subscriptions list showed many
duplicate Grok subscriptions for one Grok connection. Each Grok OAuth
token refresh added one more card.
**Expected behavior**
One Grok connection shows one subscription card. A token refresh does
not change it.
**Steps to reproduce**
1. Connect an xAI/Grok subscription account.
2. Run an agent until the Grok CLI refreshes its OAuth token. Paperclip
writes the refreshed credential back to the vault.
3. Run the agent again.
4. Open Costs → Subscriptions. A second Grok card is there. Each refresh
adds another card.
**Agent adapter(s) involved**
Not adapter-specific (core bug). The trigger is the Grok OAuth refresh
write-back in `server/src/services/ai-connection-runtime.ts`.
**Paperclip version or commit**
`master` at 8aaef59.
## What Changed
- `server/src/services/subscriptions.ts`: For an unverified identity,
`register()` now reads the grant's `ai.credential` secret ref and that
secret's `aiSessionEpoch`. The account key becomes `grant + session +
secretId + epoch`. Before, it was `grant + credential hash`.
- Refresh write-backs use `preserveAiSessionEpoch: true`, so a refreshed
token keeps the same row.
- A reconnect advances the epoch, so it still gets a separate row. A
reconnect can sign in to a different account.
- A grant with no vaulted secret ref falls back to the old
credential-scoped key.
- Unverified rows stay grant-scoped. Only a verified provider
observation can match the billing identity of a different grant.
- Stale sessions are rejected. `SubscriptionConnection` now carries the
`session` (secret id + epoch) captured together with the credential. If
that session does not match the grant's current secret and epoch,
`register()` returns `409` and does not use the live epoch. Before this,
a run could read account A's credential, pause, and register after a
reconnect to account B. It then got B's subscription id, so A's usage
was saved against B.
- `server/src/services/ai-connection-runtime.ts`: The run passes the
secret id and epoch from its existing freshness check (read together
with the credential).
- `server/src/services/subscription-refresh.ts`: Discovery reads the
session before it reads the credential. Then a reconnect at any time
after that read makes `register()` reject the stale session.
- `server/src/__tests__/subscriptions.test.ts`: Added regression tests.
- Two refreshes keep one row and its saved price. A reconnect (epoch
bump) makes a second row, and a refresh after that stays on the second
row.
- A registration with an older epoch, or with no session, is rejected
with `409`. The grant binding stays on the current account.
- `server/src/__tests__/subscription-refresh.test.ts`: The module mock
now exports `subscriptionSession`.
- This pull request adds no code comments. The explanation is in this
description.
## Verification
Run on `master` + this branch (head `11900e4`):
- `cd server && npx vitest run src/__tests__/subscriptions.test.ts
src/__tests__/subscription-refresh.test.ts
src/__tests__/subscription-routes.test.ts
src/__tests__/ai-connections.test.ts
src/__tests__/ai-connection-session.test.ts
src/__tests__/costs-service.test.ts` → 6 files, 191 tests pass.
- The refresh test fails on `master` without the service change. The
refreshed token gets a new subscription id (`expected '6683…' to be
'942b…'`).
- The stale-session test fails when the guard is removed. The stale
registration resolves to the reconnected account's id instead of
rejecting.
- `pnpm -r typecheck` passes for every package except `server` and
`cli`. The runner passes `typecheck:typescript`.
- I could not complete some checks in my sandbox. CI must confirm them:
- The `server` and `cli` typechecks hit the 4 GB sandbox memory limit
(OOM kill). A narrow `tsc --noEmit` of `subscriptions.ts` and its
imports passes with 0 errors. A narrow check of
`ai-connection-runtime.ts` and `subscription-refresh.ts` also hits the
memory limit, because they import most of the server.
- The full `pnpm test:run` hit the same memory limit.
- `pnpm build` and the runner `typecheck:rust` need `cargo`, and the
sandbox does not have it. This change does not touch Rust or the runner.
Manual check after deploy: Connect Grok and run an agent through a token
refresh. Then make sure that Costs → Subscriptions shows one Grok card.
## Risks
- Low risk. There is no schema change and no migration.
- Existing unverified rows keep their old credential-scoped keys. On the
next refresh after deploy, each existing Grok connection can get one
more row in the new key format. After that, it stays stable.
- This change does not remove duplicate rows that exist now. You can
merge them with the **Link** control on each Costs card. A separate data
repair is planned.
- `register()` does one more indexed read of `company_secrets` (by id)
when a run registers an unverified identity. Discovery does one more
read of that row for each connection. The cached-binding fast path does
not change.
- A run that loses the race with a reconnect now fails its preparation
with `409` ("The AI credential changed. Retry this execution."). This is
the same retry behavior as the existing credential-freshness guard.
Discovery logs the conflict and checks again on the next pass.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Claude Opus 5.5 (Anthropic), model ID `claude-opus-5-5`, run through
Claude Code as a Paperclip agent with tool use (shell, file edit, test
execution).
## 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 (targeted suites; the full
suite is not complete, see Verification)
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (no
documentation change is necessary)
- [x] I have considered and documented any risks above
- [ ] All Paperclip CI gates are green (pending CI)
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
(pending review)
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Amy (Opus) <noreply@paperclip.ing>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Co-authored-by: devinfoley <139239+devinfoley@users.noreply.github.com>