mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 08:53:19 +01:00
e2b337b330f2a8ea7f7ffb915cf19ca5faf712cf
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
be6bb768b1 |
fix(ui): restrict company navigation to accessible memberships (#13039)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The board selects a company before it loads that company's inbox and tasks. > - Instance administrators can list companies where they have no membership. > - The board treated that directory as a list of companies the user could enter. > - This pull request gives navigation a list based on the existing company access check. > - Users can select their companies without landing on an inbox that rejects their access. ## Linked Issues or Issue Description Fixes #6090. Refs #4855 for the related account-recovery case; this PR does not grant company membership. **What happened?** An instance administrator can select a company where they have no membership. Its inbox then shows “User does not have access to this company.” Company directory visibility and access to company contents use different rules. **Expected behavior** Company navigation should show only companies the current user can enter. A stored selection for an inaccessible company should fall back to an accessible company. A direct link to an inaccessible company should use the existing unavailable-company page. **Steps to reproduce** 1. Create Company A and Company B with separate owners. 2. Sign in as an instance administrator who belongs only to Company A. 3. Select Company B through a stored selection or a link with its prefix. 4. Observe that the board accepts the company selection, but company-scoped requests return 403. Related: #10524 lets cloud users enter additional companies where they hold memberships. This fix preserves that access and excludes companies where they have no membership. ## What Changed - Added `scope=accessible` to `GET /api/companies`, using the existing `hasCompanyAccess` predicate. - Changed the board navigation list to request that scope. Instance Access uses a separate unscoped, account-keyed directory so administrators can manage all companies. Membership edits refresh navigation. - Reject empty, unknown, and repeated scope values with 400. Directory loading errors offer a retry before access controls are shown. - Added route tests for cloud, session, board-key, local trusted, non-member, and agent access. - Added client and component tests for navigation/admin request isolation, grants outside the navigation list, self-membership refresh, directory failure recovery, and forbidden administration. - Updated the API guide and OpenAPI document. ## Verification - Latest commit: 36 focused UI tests passed. The broader UI shard passed all 281 files / 2,533 tests after correcting an asynchronous test assertion. - Server authorization and OpenAPI regression suites: 31 tests passed. - UI typecheck and `pnpm check:token-gates`: passed. - `pnpm -r typecheck` and `pnpm build`: passed after review fixes. - Full local test runner: exercised the supported shards. Several unrelated suites hit embedded PostgreSQL startup failures or startup timeouts under local load. The UI regression issue found in the broad run was corrected and its full UI shard passed. These local limitations are not reported as a green full-suite result. - [GitHub CI](https://github.com/paperclipai/paperclip/actions/runs/34233416473): all checks green on `4e1698cd4` — all server and workspace test shards, all browser end-to-end shards, typecheck/release registry, build, canary dry run, policy, and Docker context integrity. Security checks also passed. - Greptile: 5/5 on the latest commit; both initial findings addressed and all review threads resolved. ## Risks - The UI now excludes companies visible only through instance administrator status. Company membership continues to control access to contents. - Additional companies with active memberships remain available. - The client and server changes must ship together. An older server ignores the new query parameter and retains the previous behavior. - No database migration or permission grant changes. ## Model Used - OpenAI GPT-6 through Codex, with reasoning, repository inspection, code editing, and test execution. The exact served model identifier and context window 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> |
||
|
|
327f59cac2 |
refactor(ui): key the company list by account (#11488)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Which companies a person belongs to is an authorization fact the server owns, and the UI caches the answer for speed > - It cached that answer under one `["companies"]` key with no account attached, while `main.tsx` sets `staleTime: 30_000` for every query > - So for thirty seconds after an account change, one person's list answered questions asked about another, arriving with no loading state and no error > - Three separate consumers each grew their own defense against this, and each was a place to forget one > - This pull request keys the entry by account, so a list belonging to someone else is not distrusted but unreachable > - The benefit is that the protection stops depending on every future consumer remembering to defend itself ## Linked Issues or Issue Description No public issue exists. Refs #11380, #11382, #11417, #11430. The problem follows. **What happened?** The company list lived in a single cache entry, `["companies"]`, carrying no record of which account it was fetched for. Combined with the app-wide 30s `staleTime`, any read within that window after an account change returned the previous account's list — from cache, with no request, no loading state and no error. Every consumer that treats the list as an authorization fact had to know this and defend itself: - `InviteLanding` reads it to decide whether you already belong to the inviting company (#11417). - `OnboardingWizard` reads it to decide whether a saved draft belongs to you (#11382, merged). - `CompanyProvider` reads it to pick and persist your active company (#11430). All three defenses are correct. The problem is structural: the fourth consumer has to invent a fourth one. **Expected behavior** A cached company list can only answer questions about the account it was fetched for. **Steps to reproduce** 1. On a self-hosted instance in `authenticated` mode, sign in as account A, which belongs to company X. 2. Within thirty seconds, have account B become the session in that tab — a second tab signing in, or A's session lapsing server-side. 3. Any consumer reading the company list receives A's list, and nothing in the query result indicates it is not B's. **Paperclip version or commit** `master` at `ac91b7f3b`, which includes #11430. ## What Changed - `ui/src/lib/queryKeys.ts` — `companies.list(userId)` replaces `companies.all` as the list's entry. `companies.all` remains the prefix, so it still matches for invalidation. - `ui/src/api/companies-query.ts` — `companyListQueryOptions(userId)` builds the keyed options; `useCompanyListQuery()` is the only observer entry point and holds until the session settles, because the key cannot be built before then; `fetchCompanyListForCurrentAccount(queryClient)` covers imperative paths; `useAccountIdentity()` exposes the session identity the key is built from. - `ui/src/api/companies-query.ts` — the `/companies` detach moved into the query function. - `ui/src/context/CompanyContext.tsx` — drops the session-watching refetch machinery the key now makes unnecessary (`removeQueries`, the explicit replacement fetch, the awaiting gate). It still clears the live selection on an account change, because that is component state and does not change key with the query. - `ui/src/pages/InviteLanding.tsx`, `ui/src/components/OnboardingWizard.tsx` — read through the account-aware API. - Tests — the account-keyed guarantee, prefix invalidation still reaching the list, the detach inside the query function, and updates where suites seeded the old shared key. ### The existing defenses are deliberately left in place The per-consumer gates in #11382, #11417 and #11430 are now belt and braces. They are also what will catch this refactor if it is wrong somewhere, so removing them in the same change that moves the foundation would be the wrong order. Simplifying them is a follow-up, once this has proven itself. ### Why `retry: 1` appears in CompanyProvider An earlier measurement on #11430 found a retry on the replacement fetch changed no outcome, because `removeQueries` made the observer rebind and issue a second request for free. Keying by account removes that mechanism and the free attempt with it. The retry now carries the property the incidental refetch used to — a single blip during an account change should not leave the customer with no companies until they find "Try again". #11430's test for that property is unchanged and still passes, which is how the gap was caught. ### A regression this went through, kept for the record Gating the query on the session settling meant that while the account was unknown the query was *disabled*, and a disabled query reports `isLoading: false` with no data — which the provider defaults to an empty list and reads as "asked, and owns nothing". That is the destructive branch #11477 had just fixed, reached through a different door: it would have cleared the customer's stored company on every cold boot. #11477's test caught it during the rebase. `useCompanyListQuery` now reports the wait for the account as part of the wait for the list. ### What this does not do It does not scope the rest of the per-account cache. `["companies", id]`, stats, and every other account-scoped entry still survive an account change; that is the cache-lifetime work in #11380. ## Verification - `pnpm vitest run` in `ui`: **4018 passed, 1 failed**. - `pnpm tsc -b` in `ui`: clean. - `companies-query.test.ts`: 6 passed. `CompanyContext.test.tsx`: 17 passed. `OnboardingWizard.test.tsx`: 13 passed. `InviteLanding.test.tsx`: 13 passed. The failure is the pre-existing timezone-dependent `IssueProperties.test.tsx`, fixed by #11478. Two behaviours are asserted rather than assumed, because the refactor is only safe if they hold: that invalidating the `companies` prefix still marks the account-keyed list stale (19 call sites depend on it), and that the query function detaches the in-flight `/companies` request before fetching. **Not done:** no manual two-account run in a browser. The path needs two accounts on an `authenticated` instance, which a local dev instance cannot exercise. ## Risks Moderate, and worth reading before approving. **It touches `InviteLanding.tsx`, which #11417 also modifies**, so one of the two will need a rebase — the conflict is mechanical (both change how the same query is read). This was #11481, which GitHub closed automatically when its base branch (#11430's) was deleted on merge; reopening a pull request whose base branch is gone is not permitted, so it continues here against `master` with the same head and the same review already recorded on #11481. **The list now waits for the session query.** The key cannot be built before the account is known. In the app the session is already fetched at boot by many components, so this is a dependency rather than an extra request, but it does serialize: on a cold boot the list waits for the session to land. Every test that renders a company-list consumer now needs a session in the cache, which is why several suites gained a seed. **A missing mock surfaces as a passing gate rather than an error.** The detach inside the query function meant suites whose `companiesApi` mock lacked `detachInflightList` had their query function throw, which read as "decided" in the onboarding gate and mounted the wizard early. Fixed in the affected suites; worth knowing as a failure mode. ## 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> |
||
|
|
a116713b9a |
feat(mcp) [split 6/8]: add Tools and Profiles UI foundation (#9561)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Governed MCP access spans contracts, runtime enforcement, adapters, UI surfaces, and operator verification > - The parity reference PR #9534 is too large for effective automated or human review > - The feature therefore needs a linear stack whose individual diffs stay below the 100-file review limit > - This pull request is split 6/8 and focuses on UI API, shared components, and Tools/Profile surfaces > - The benefit is a standalone, testable review boundary while preserving byte-for-byte parity at the top of the stack ## Linked Issues or Issue Description - Related parity reference: #9534 - Problem: Operators need typed clients and administration surfaces that compile independently before navigation exposes them. - Proposed solution: Adds UI APIs, hooks, libraries, shared components, Tools/Profiles pages, and the plugin settings consumer required by the new company-scoped API. - Alternatives considered: keeping #9534 as one 403-file review, or rewriting the feature to manufacture seams; both were rejected in favor of path extraction plus compile-driven boundary moves. - Roadmap alignment: this advances the existing governed MCP/tool-access work already represented by #9534; it does not introduce a separate roadmap initiative. - Stack position: base branch is `pap10341-split/05-runtime-integration`. - Merge policy: merge bottom-up, in order, only after the complete eight-PR stack has been reviewed and the top-of-stack parity gate remains empty. - Requested review: UXDesigner sanity pass on Tools/Profile surfaces; Greptile on every PR. ## What Changed - Adds UI APIs, hooks, libraries, shared components, Tools/Profiles pages, and the plugin settings consumer required by the new company-scoped API. - Keeps this PR below 100 changed files and independently typecheckable. - Preserves the final tree from #9534 when combined with the other seven stack levels. ## Verification - `pnpm typecheck` - `pnpm check:token-gates` — all gates clean - Focused UI Vitest run with `NODE_ENV=test` — 28 files, 183 tests passed ## Risks - Large dead-code UI additions can drift from activation routes; PR 7 supplies the registration layer and top-level parity catches omissions. - Stack risk: merging out of order can expose incomplete layers; mitigate by following the documented bottom-up merge policy. - Parity risk: later edits to an intermediate branch can drift from #9534; mitigate by re-running the empty top-of-stack diff before merge. > 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, exact model ID `gpt-5.4`; runtime-managed context window; medium reasoning with repository, shell, Git, GitHub CLI, and code-execution tools enabled. ## 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] Internal references are omitted except the execution-plan link explicitly required for this coordinated split stack - [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 ## Stack Coordination - Internal execution plan: [PAP-13874](/PAP/issues/PAP-13874#document-plan) - Parity reference: #9534 - Stack: #9556 → #9557 → #9558 → #9559 → #9560 → #9561 → #9562 → #9563 - Merge bottom-up only after full-stack review and an empty parity diff at #9563. ## UI Evidence QA captured these from the live Garden MCP split stack at 1440px and verified clean rendering:    --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |