Commit Graph

4 Commits

Author SHA1 Message Date
Tonio 6d0adbfb5d
refactor(ui): retire the shared-cache reasoning the account key made obsolete (#11507)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work
> - Three places in the UI turn the company list into an authorization
verdict: the invite landing page, the onboarding draft gate, and company
auto-selection
> - Each grew a defense when one `["companies"]` cache entry answered
for every account, and each documented that hazard at length
> - #11488 keyed the entry by account, so the hazard those comments
describe can no longer happen
> - The comments stayed, and a comment that describes a trap that no
longer exists is how the next reader removes a mechanism that is still
holding something up
> - This pull request replaces that reasoning with what the mechanisms
actually do now, and removes the one condition that genuinely went dead
> - The benefit is that the next person to simplify these gates has
accurate reasons to work from

## Linked Issues or Issue Description

No public issue exists. Follow-up to #11488, #11430 and #11417. The
problem follows.

**What happened?**

The three gates were written against a shared, account-less company
cache. #11488 keyed that entry by account, which made the documented
hazard impossible — but the documentation stayed. Each gate now carries
a long explanation of a cross-account leak that the key prevents, while
the mechanism it explains is in fact still required for a different and
unrelated reason.

That is a maintenance hazard in a specific direction: a reader who
checks the comment against the code concludes the mechanism is obsolete,
removes it, and reintroduces a failure the comment never mentioned.

**Expected behavior**

The reasoning next to each gate describes why the gate is there now.

**Steps to reproduce**

Read the comment above `ownershipDecidable` in `OnboardingWizard.tsx`
against `master`. It justifies `isSuccess` on the grounds that "after an
account switch the retained value is the previous account's list", which
the account-keyed entry makes impossible.

**Paperclip version or commit**

`master` at `0817fbad9`.

## What Changed

- `ui/src/pages/InviteLanding.tsx` — dropped the
`Boolean(sessionQuery.data)` conjunct from `membershipListIsCurrent`;
rewrote the comment.
- `ui/src/components/OnboardingWizard.tsx` — replaced the shared-cache
explanation above the ownership gate with the reason the gate still
exists.
- `ui/src/hooks/useSignOut.ts` — corrected the sweep's rationale, which
cited the company list as its example of data the next account could
read.

### The one dead condition

`membershipListIsCurrent` tested `Boolean(sessionQuery.data) &&
companiesQuery.isFetchedAfterMount`. The first term cannot be false when
the second is true: the query is `enabled` only while a session exists,
so the flag cannot be set without one. The lapsed-session case it looked
like it covered is covered by the keying instead — the observer re-keys
to the anonymous entry and holds no data to leak.

Tests pass with it removed, but that only shows no test distinguishes
it, which is why the reasoning above is recorded in the code rather than
left for the next reader to redo.

### What is deliberately kept

Each gate turned out to be load-bearing for a reason that has nothing to
do with accounts:

- **InviteLanding** still waits for a list fetched this mount. A pending
query reads as an empty list, which reads as "not a member", which
auto-accepts an invite the customer may already hold.
- **OnboardingWizard** still forces a fetch with `staleTime: 0`. A
cached list is the right account's but can be thirty seconds old, so a
company created moments ago in another tab is missing from it — and
missing reads as "you do not own this", which *deletes* the draft rather
than withholding it.
- **CompanyProvider** still clears the live selection on an account
change. That is component state and does not change key with the query.

Removing them as redundant is the mistake the stale comments invited;
this change is what makes that argument harder to make by accident.

## Verification

- `pnpm tsc -b` in `ui`: clean.
- `InviteLanding.test.tsx`, `OnboardingWizard.test.tsx`,
`CompanyContext.test.tsx`, `useSignOut.test.tsx`,
`companies-query.test.ts`: **67 passed**, run twice.

No behaviour change is claimed and none is intended: the only
non-comment edit is the removal of a condition that cannot alter the
expression's value.

**Not done:** no browser run. Nothing here is observable at runtime.

## Risks

Low. Comments, plus one condition shown to be unreachable-false.

The risk that remains is a documentation risk in the other direction: if
the keying is ever reverted or bypassed, these comments will understate
what the gates protect against. They name #11488 so that connection is
findable.

**This does not close the class.** Account-scoped entries other than the
list — `["companies", id]`, stats, and the rest — still survive an
account change that skips the sign-out button. That is unclaimed work,
and larger than this.

## Model Used

Claude Opus 5 (`claude-opus-5`), through Claude Code. Extended thinking
enabled. Tool use enabled: file read and edit, shell for typecheck and
test runs.

## 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 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
- [ ] All Paperclip CI gates are green
- [ ] 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: Claude Opus 5 <noreply@anthropic.com>
2026-08-16 22:34:55 -07:00
Tonio 0023c5c4a6
refactor(ui): record why sign-out resets rather than removes (#11473)
Comment only. Salvaged from `claude/sign-out-cache-note`, written while #11380
was in progress and never opened as a PR; that branch is deleted with this.

#11380 landed the reasoning for `resetQueries` but not the evidence, and not
the part that stops someone reversing it later.

The choice was measured rather than argued. Against query-core 5.101.4,
removal produced 0 notifications and left the observer holding the signed-out
account's session; reset produced 3 and null. The consequence of the former is
not only a stale read - CloudAccessGate's redirect fires on the session going
empty, so it never runs.

The opposite advice really does hold for a local reset of a key an observer is
still mounted against: reset rewinds the update counters `isFetchedAfterMount`
derives from while that observer keeps its bind-time baseline, so anything
gated on the flag withholds forever. Sign-out is not that case, because it
resets the session too and the consumer unmounts on the redirect.

Two sessions reached opposite recommendations on this API within a day, both
correct about different situations, which is the kind of thing a later reader
re-litigates without a note in the file.

The first revision of this note named the wrong consumers - InviteLanding and
the onboarding draft gate, neither of which reads `isFetchedAfterMount`.
`AppsConnect` is the only one on master and is what it names now.

ui typecheck clean; 13 pass across the sign-out suites.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-16 09:44:36 -07:00
Tonio 38752c4e5e
fix(ui): clear account-scoped query caches on sign-out (#11380)
Sign-out invalidated two query keys - the auth session and health - and left
everything else in the cache. `invalidateQueries` also keeps serving the old
data while it refetches, so it was insufficient even for the two it did touch.
The company list was never touched at all, which is how one account's
companies could still be in hand when the next account signed in.

The self-hosted path now resets every account-scoped entry. Scoping is an
allowlist of instance-scoped roots rather than a list of things to clear, so a
query key added later is account-scoped unless someone deliberately says
otherwise - forgetting this file fails closed.

`health` is the only exemption. It carries deployment mode, bootstrap state
and Cloud metadata, nothing account-scoped, and `useCloudInstance` observes it
with `enabled: false` and leaves the fetch to CloudAccessGate. Dropping the
entry would strand every such observer on `null` until the gate happened to
refetch, flipping Cloud instances into their self-hosted rendering mid
sign-out. It is refreshed in place instead.

`resetQueries` rather than `removeQueries`: removal empties the cache without
notifying the observers already subscribed, so a mounted `useQuery` keeps
returning its last result until an unrelated re-render rebuilds it.
CompanyProvider sits above the router and stays mounted across the whole
sign-out and sign-in cycle, so that is the common case here rather than a
corner one. Reset notifies them, so the old data leaves the cache and
everything reading it.

The cloud path is untouched: it is a top-level navigation, and the document
reload builds a new QueryClient with nothing left to clear.

This is the root cause behind the onboarding draft-ownership gate added in
#11382. That gate stays, and its comment now says why: this fix covers the
sign-out button, not the question. An account can change without it - a
session lapsing server-side, a second account signing in on a warm tab, a
caller supplying the company context from somewhere else - so the gate stays
independent rather than deferring to this.

ui typecheck clean; full ui suite 4014 pass, with only the timezone-dependent
IssueProperties failure already present on master.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-08-15 07:55:14 -07:00
Dotta f258b34bbd
fix(ui): unify cloud-managed sign-out (#10994)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Paperclip can run as a self-hosted app or as a Cloud-managed tenant.
> - These modes need different sign-out sequences because Cloud owns
three sessions.
> - Several visible controls implemented sign-out separately and could
choose different paths.
> - This pull request adds one Cloud-aware sign-out action and moves
every visible control to it.
> - The benefit is one safe sign-out path in Cloud and unchanged local
sign-out in self-hosted deployments.

## Linked Issues or Issue Description

Refs #2073. That older PR adds a separate company-settings sign-out
surface. This change centralizes the existing account, company, and
instance-settings surfaces and preserves the self-hosted behavior
described there.

**What happened?**

Visible sign-out controls used separate implementations. A Cloud-managed
control could call the app-local sign-out endpoint and open the local
auth page. That path did not enter the Cloud-owned logout sequence for
the tenant, Cloud, and identity sessions.

**Expected behavior**

Every visible sign-out control must use one action. Cloud-managed
instances must navigate the top-level window to the same-origin
`/cloud/logout` route without a local sign-out call first. Authenticated
self-hosted instances must keep the local sign-out API and cache
invalidation behavior.

**Steps to reproduce**

1. Open a Cloud-managed tenant.
2. Use the account menu, company menu, or instance-settings sign-out
control.
3. Observe that independently implemented controls can enter different
sign-out paths.

**Paperclip version or commit**

The problem reproduces at `656ecfa585b31938e2685ffab3db22e794474803`.

**Deployment mode**

Cloud-managed tenant built from source. The regression tests also cover
authenticated self-hosted mode.

## What Changed

- Added `useSignOut` as the shared Cloud-aware sign-out action.
- Navigated Cloud-managed sessions to `/cloud/logout` exactly once
without calling local auth first.
- Preserved local API sign-out and cache invalidation for authenticated
self-hosted sessions.
- Migrated the account menu, company menu, and instance general settings
to the shared action.
- Added focused tests for mode selection, menu closure, pending state,
failure state, and settings behavior.

## Verification

- `pnpm exec vitest run ui/src/hooks/useSignOut.test.tsx
ui/src/components/SidebarAccountMenu.test.tsx
ui/src/components/SidebarCompanyMenu.test.tsx
ui/src/pages/InstanceGeneralSettings.test.tsx` — 26 tests passed.
- `pnpm --filter @paperclipai/ui typecheck` — passed.
- `pnpm check:token-gates` — passed.
- `git diff --check origin/master..HEAD` — passed.

## Risks

- Low risk. The change centralizes existing behavior and adds no schema,
API, telemetry, or style-token changes.
- Cloud mode depends on the existing health decision. Tests pin both
mode branches.
- The change does not alter Fetch Metadata, CSRF, cookie, prefetch, or
return-URL protections owned by the Cloud logout route.

> 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

- OpenAI Codex, GPT-5. The runtime does not expose a more specific model
ID or context-window size. The agent used high-reasoning mode, shell
tools, and API tools.

## 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
- [ ] 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>
2026-08-06 12:13:14 -05:00