mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-04 02:32:07 +01:00
ab0b15d234ef9bfbb3728fa97a5e2fe9a8d0c1de
3
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandPaperclip
|
f1d7808382 |
fix(tools): stabilize tied profile entry order (#18427)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The Advanced YAML editor reads shared tool profiles through their owning service. > - The editor must preserve hidden settings and reject real concurrent changes. > - Batch-created entries share a creation time. Queries sorted only by that time can return the same entries in a different order. > - The editor can then report a false concurrent tool change before it sends a profile binding request. > - This pull request adds an ID tie-breaker to profile entry reads. > - The benefit is a stable saved configuration without removing the concurrent-change guard. ## Linked Issues or Issue Description Refs #18356. Related: #18168. **What happened?** The compact YAML browser case failed at its second save. Save stayed enabled and the profile binding write did not occur. The original CI artifact has no response bodies. It does not establish the exact entry order from that run. A controlled browser test reproduced this failure. It changed only the order of 30 equal-time entries in the second preflight response. The client reported `tools changed while you were editing. Reload saved before saving.` No binding request followed. A separate first-save response barrier showed that the pending-save guard worked. **Expected behavior** Reads of unchanged entries must have a stable order. Real entry changes must still trigger the existing configuration checks. Entry order does not set policy precedence; exclusions still win over matching includes. **Steps to reproduce** Create a profile with entries that share one creation time. Read the profile, change the physical row layout in a disposable PostgreSQL database, and read it again. The old service has no tie-breaker for these entries. The new regression fails against that service. ## What Changed - Sort list and detail entry reads by creation time, then ID, in the profile service. These reads supply saved configuration snapshots. Other reads retain their existing behavior. - Add one real PostgreSQL regression. It changes heap layout in both directions, checks stable list and detail values, preserves chronological ordering, checks company boundaries, and checks include/exclude results and a real entry change. - Include the exact test-only recovery prerequisite from commit `36c8d9ddd76d0b7ddb28a9a6c674d9e538f6df88`. Its file blob is unchanged. It removes a one-sweep fixture assumption while keeping bounded recovery assertions. No document UI or journey code is imported. ## Verification - Base: one frozen public master snapshot, `8c9dd1e4445e58cc9ca08076dfe9c0b27038d6ee`. The affected service, schema, projection, resolver, and save guard match the failed CI merge `94d98d5c75c8f331aad4bfab2111ee587b0c63fe`. - RED: the new real PostgreSQL test fails on the old service. Both physical layouts were verified before the ordering assertion failed. - GREEN: `cd server && pnpm exec vitest run src/__tests__/tool-profile-entry-order.test.ts src/__tests__/tool-profile-entry-validation.test.ts` — 18 tests pass with real PostgreSQL 17. - GREEN: `cd ui && pnpm exec vitest run src/lib/agent-configuration/save.test.ts src/lib/agent-configuration-yaml.test.ts --maxWorkers 1` — 51 tests pass. - GREEN: one focused Chromium treatment of the original compact YAML case passes. The treatment changes the actual disposable database heap after the first save. It does not transform HTTP responses. The 30 equal-time entries stay identical through the API, the second bind returns 201, and the original hidden-setting and disabled-Save assertions pass. - `node scripts/check-maps.mjs` and `git diff --check` pass. - Local tests use Node 24, a macOS source server, Chromium, and an isolated loopback PostgreSQL 17 container. The server and container are stopped. Private traces and control source are retained outside Git. These controls do not prove a deployed Cloud result. - A review follow-up removes two unnecessary query changes. Fresh real PostgreSQL verification still passes all 18 tests. The list/detail queries and browser path are unchanged from the passing heap-reorder treatment. - Prior head `59edcade203d6a05923955f5a0118e62c7e549c5` had a separate browser failure in run `41169427774`: `disconnect-write-recovery.spec.ts:204` kept the original title after a mocked session lookup failure. Its relevant test and UI source are unchanged from the base. The cause is unresolved. The two removed query changes do not repair it. The original log and artifact are retained; no unchanged job retry was issued. On the current head, this case passes in 14.3 seconds. The compact YAML case also passes in 11.2 seconds. The later success does not establish the prior failure cause. - Final exact head: `81c584aeac116b9d7a126b55dc99d5f6c70c195f`. [CI run 41169978784](https://github.com/paperclipai/paperclip/actions/runs/41169978784) completes with success: all 47 workflow jobs are terminal, and all 57 PR check/status entries are green or accepted neutral/skipped states. Full typecheck, build, Runner checks, security checks, and `ci / verify` pass. They were not repeated as broad local commands because the shared host is under load. - The new PostgreSQL regression also passes in Linux CI in 911 ms. Server shard 12 has 1,839 passing tests. Browser shard 3 has 45 passing tests, including the original compact YAML case. - Fresh Greptile and Paperclip reviews both give this exact head 5/5 with no findings. All review threads are resolved. The branch is mergeable and clean. ## Risks - Entries with the same creation time now use ID order. Their prior relative order was undefined. Entries with different creation times keep their existing order. - The query still sorts the same rows and adds one key to the sort. It adds no query, polling, schema change, or migration. - The configuration equality and concurrent-change checks are unchanged. This fix does not ignore real tool changes. - The original CI response order is unknown. The regression proves a permitted ordering defect and the matching failure mechanism, not the unique historical cause. ## Model Used - OpenAI Codex, based on GPT-6, with reasoning, source inspection, terminal tools, and browser 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> |
||
|
DottaandPaperclip
|
13709b4c7a |
feat(connections): let agents configure connections within delegated authority (#17737)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Connections provide governed tools, chat channels, model accounts,
and gateways.
> - Several management routes require a human board actor even when an
agent has the required grant.
> - Default agents must configure these resources within the responsible
user's authority.
> - This pull request adds a shared authorization path to the existing
REST and native API surface.
> - Human authentication, execution approvals, and local host boundaries
remain enforced.
> - The benefit is that a standard agent can complete authorized setup
with a bounded human handoff.
## Linked Issues or Issue Description
**Subsystem affected**
Server authorization, connection services, shared contracts, native API
discovery, database schema.
**Problem or motivation**
Connection administration is available in the web app but often
unavailable to agents. Agent grants, responsible-user authority,
credential ownership, and actor attribution need separate checks.
Opening gateway-token creation also requires preserving the issuing
agent's delegated authority during token use.
**Proposed solution**
Use the existing authorization evaluator for every connection family.
Enforce current responsible-user authority, effective trust, grant
scope, resource visibility, and target-agent access. Keep existing agent
grants unchanged. Expose authorized configuration through default
`search_api` and `call_api`. Save human authentication steps and let
agents continue after completion.
**Roadmap alignment**
This change completes the API access path for the existing Connected
Apps, Personal & Shared AI Accounts, and governed MCP Gateway
capabilities in `ROADMAP.md`. It follows the maintainer's requested
design. Related work: #11899 enforces connection execution access and
creator/manager rules; this PR extends agent management across
connection families. It does not replace that execution-policy work.
## What Changed
- Add shared connection management context and mandatory
responsible-user, trust, scope, and target checks.
- Allow authorized agents to configure tool, chat/email, GitHub/Slack,
AI, profile, and gateway resources through existing routes.
- Preserve human credential ownership and record the actual agent,
responsible user, and run or key in audit data.
- Preserve saved browser authentication handoffs and callback session
checks.
- Bind agent-issued durable gateway tokens to their responsible user and
recheck delegated authority at use.
- Add explicit REST transport and management capability metadata,
built-in guidance, and sensitive-response handling.
- Intersect caller and endpoint-agent Slack test/invitation policies,
including independent and shared rates. Require the addressed human’s
current endpoint identity link before completing identity handoffs.
- Add production-authenticated integration scenarios. Companion #17739
contains the saved setup UI (including the required human
import/rotation step), product and permission documentation,
catalog/Vercel credential and callback ownership, and operation maps to
keep each review below 100 files. Both PRs must ship together; merge
this implementation first, then its companion.
## Verification
- Current implementation head:
`2ecd6451d0d0c8d82121a4e3fa4ce94c5befbbd6`, including master
`ea727dc3c2`, 99 files. The latest follow-up fixes the two valid
findings from the preceding f9 review. Exact-head Greptile has completed
at5/5 with no findings and Superagent passed. Full exact-head Linux CI
workflow40446606056 completed SUCCESS; only the queued external
Paperclip Review remains pending; earlier Linux passes do not qualify
this successor.
- Latest preserved-stack verification passed **2,165 tests** in fresh
PostgreSQL: **70 actual-auth scenarios**, 27 OpenAPI contracts, 2,022
native API tests and 46 saved GitHub wizard cases. **141 existing
gateway/policy acceptance tests** also passed. Workspace
typecheck/build, maps and token gates passed. The first workspace
typecheck was blocked by sandbox tsx IPC permissions; its authorized
rerun passed. This is focused local evidence; the full Linux CI matrix
is still required.
- Issuer/target gateway policy regressions were red before the fix. Both
decisions now intersect, including Block/Ask first/rates, same
credential identity and current token/user/agent authority after
approval. Invocations retain their token and actual requesting agent;
deleted tokens fail closed. Thirteen actual-auth cases cover revocation,
narrowed scope, approvals, shared rates and board compatibility.
Intermediate missing-token attribution, review-context and
implicit-session actor failures remain in the qualification record; all
final cases pass.
- Activity pagination now retains a bounded cursor across private-only
windows. The actual-JWT regression reaches three older readable events
after 27 private ones without exposing private details. A native
device-login regression was red for plaintext receipts; local check and
test-call results now use existing protected-response handling. The
passing actual-auth case proves encrypted persistence, exact replay,
redacted history and no response asset.
- Previous-head `5b55195593` Linux CI initially failed e2e shard 4/8 in
the existing connection-intent continuation test: periodic recovery
queued a repair at 01:21:47, the scripted journey completed the task at
01:21:48, and the extra queued run was cancelled at 01:21:49 because the
task was terminal. The assertion selected that latest cancelled run.
GitHub refused the first attempt while the workflow was nonterminal.
After completion, the single evidence-supported failed-job rerun was
accepted as **attempt 2** on the same head (workflow 40431202466). The
failed shard and final E2E aggregate passed on attempt 2, and the
complete Linux workflow for that head is successful. The original
failure remains part of qualification. No additional unchanged-code
rerun will be used for this failure.
- Companion CI also has an unchanged login-cleanup idleness timeout,
retained in its qualification record. That same 16-case suite passed in
this parent Linux run and a fresh local database. Both preceding
workflows passed after their single failed-job reruns as attempt 2; the
original failures remain recorded. No additional unchanged-code rerun
was used.
- Migration `0380_broken_aqueduct.sql` was regenerated through the
repository workflow after master took 0379 for project Git
authorization. It adds the responsible-user column with `IF NOT EXISTS`,
preserves upstream migrations, and satisfies snapshot retention and
migration safety checks.
- Upstream published Copilot v62 and v63 identities remain byte-for-byte
unchanged. The combined guidance is now v64, digest
`sha256:22e31f95ef972bcead98a4a707c9f33433558fd111e390e027a2bbb4346e112a`.
The normal shared TypeScript/Rust identity check and **61 Copilot
installation tests passed**. The old source-identity assertion first
failed against unchanged published v63; regenerating a new v64 preserves
the published release.
- Workspace `pnpm -r typecheck` and `pnpm build`, map validation, and UI
token gates passed on the final combined stack after the upstream merge.
Node 24.22.1 / pnpm 9.15.4 were used.
- **Earlier 82 API/OpenAPI tests passed** in fresh PostgreSQL 18: 55
assembled-server scenarios and 27 contracts. They use real persistent
agent keys, run JWTs, human sessions, production defaults, and
deterministic provider fixtures. No actor injection establishes access.
- The new email regression was red before per-account scope filtering.
The Arcade regression was red before human ownership and inventory-key
fixes; it also verifies actual-agent audit identity and a human-session
inventory read. The activity regression measured **200 authorization
checks before the fix and one afterward** for the same one-item page.
- **82 GitHub UI tests passed**, including all three new
failed-replacement regressions that were red before their fix. The UI
preserves an earlier copy only after confirming its committed
generation. Changed or unreadable results block copying and reconnecting
until reconciled. Human-only secret responses now include their
nonsecret committed generation.
- Private test status now rechecks the originating principal and linked
task, including mandatory agent/responsible-user privacy. Cross-agent
tests require the full caller tool access decision and the same
credential identity, regardless of auth label; caller and target
policies/approvals and rate limits apply before execution and after
human approval. A profile-only precondition was found to reject valid
allow-policy and scoped-grant access and has been removed. Both
permitted-access cases and a no-auth-label identity regression were red
before the correction; caller-specific Block still prevents provider
execution. The private-status regression was also red before its fix.
- The tool-activity regression measured 100 capability decisions and 102
run captures on a 100-event page before the fix. It now uses two task
decisions and at most two captures while retaining the same 50 visible
events, private-event filtering, and 403 after audit-grant revocation.
The regression was red before request-context reuse and task-decision
sharing; final server typecheck/build also pass.
- Generic chat configure/reconnect now hands agent-supplied verification
credentials to the addressed human before provider work or vault
changes. Actual key/JWT regressions were red for Slack signing-key
replacement and generic channel secret-reference rebinding. They now
prove unchanged refs and vaulted signing value, human setup, permitted
agent bot-token repair, and governed test messages. All 82 API/OpenAPI
cases pass in fresh PostgreSQL. Eight native receipt protections and
final server typecheck/build also pass.
- The preceding review found that old healthy state could retire a
credential-entry card before a human changed anything. The actual-JWT
regression was red. Saved field generations now change only with
protected values and commit with refs plus a verification fence; retries
retain the baseline. Old health, bot-token repair and unchanged entry
stay pending. New signing verification and current authority permit
continuation. A separate red race proved that user revocation during
provider authentication could return 403 after refs changed; authority
is now checked before persistence and inside the credential transaction.
The final 82 API/OpenAPI cases, four neighboring real-database
Slack/GitHub lifecycle cases, and final server typecheck/build pass.
- A preceding full run crossed a rate-window boundary. Database rows
prove one consumed slot at 23:35 and another at 23:36 for the same
policy/bucket. Only Date is frozen in the rate test cases; transport and
asynchronous timers remain real. Production rate behavior is unchanged,
and the final full run passes. An intermediate fixture failure that
overwrote hidden generation metadata with an API projection is retained
in the qualification record.
- Linux CI on an earlier head found two rate-admission assertions that
omitted the newly persisted bucket key. The failures were reproduced
locally; both assertions now verify the exact actor/tool bucket and all
49 policy-service tests pass. The companion qualification record retains
the initial and intermediate failures. Catalog creator attribution is
preserved in the companion to keep this implementation review at 99
files.
- The latest review reproduced three regressions before repair: complete
human replacements failed on unavailable old signing secrets, fresh
GitHub links incorrectly selected reconnect, and different
credential-entry requests shared an idempotency key. Each old protected
ref now has a narrow known-unavailable fallback; unexpected resolver
failures and company/scope violations still abort. Fresh imports keep
their credential form, configured endpoints keep reconnect, and
canonical field sets/completion modes distinguish handoffs while
matching retries retain their baseline. All 55 actual-auth scenarios and
27 separately run OpenAPI contracts pass, as do server typecheck/build.
Companion #17739 also passes all 41 wizard tests including actual
fresh-form submission, UI typecheck, maps and token gates. Earlier red
results and a mistyped OpenAPI filename are retained in qualification.
- The configured GitHub handoff regression was red in the
browser-component tests and actual-auth link/persistence tests. Saved
links now name requested fields and all/any completion mode; the
companion form submits those supplied fields through reconnect and
retains other saved credentials. Webhook/client-secret reconnect remains
human-only; first-setup secret restrictions remain. All 46 wizard tests
pass, including combined modes and legacy links. A duplicate review
referenced the older companion bf760912; the current form is already
fixed, linked in the reply, and its thread is resolved.
- Linux CI on
|
||
|
DottaandPaperclip
|
233753e577 |
refactor(tools): isolate profile ownership (#17155)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Tool profiles select the actions each agent can use. > - Profile reads, review, mutation and assignment resolution shared a large OAuth and connection service. > - Those paths have their own company checks, lock order and database projections. > - This pull request gives profiles explicit owners while preserving the existing transactions and public API. > - The benefit is a smaller coordinator and a reviewable access boundary. ## Linked Issues or Issue Description Depends on #17147. Stack: #17144 → #17146 → #17147 → this PR. These PRs remain open. **What existing behavior does this improve?** Maintenance of tool profile management and effective agent assignments. **Current behavior** The service entry point contains 19,293 lines. Profile behavior is spread across its closure and returned object. **Proposed behavior** Profile management accepts only a database and clock. Assignment resolution accepts three named capabilities. Projection and shared lock functions have explicit homes. Public methods and responses remain unchanged. **Reason and benefit** Reviewers can follow profile transactions, lightweight reads and precedence without navigating OAuth setup. The entry point falls to 17,752 lines, a reduction of 1,541 lines in this PR. ## What Changed - Extract profile management, projections and effective assignment resolution into three modules. Their sizes are 886, 268 and 216 lines. - Move shared actor/transaction types, identifiers and assignment locks into named modules. Preserve the public lock export. - Keep all database reads, transaction boundaries, lock order and method signatures. Add no cache or background work. - Record the next ownership cuts toward an entry point below 5,000 lines. ## Verification - Before and after: 466 cases in `tool-access-service.test.ts` and 33 cases across profile entry validation, effective assignments and catalog publication. All 499 cases pass with one database worker. - Server TypeScript check and `node scripts/check-module-boundaries.mjs` pass. `git diff --check` passes. - Normalized syntax comparison finds all 329 pre-existing function/method bodies unchanged. Wiring passes the unchanged public service tests. - The previous stack passed full typecheck and build. Its full local test run was interrupted and remains unverified; its complete CI matrix passed. This PR will record its own current-head CI result below. - Current-head CI is green. Current review: Greptile is 5/5 at `0f5c85905e`, with no inline review threads. - CI failure preserved: [general server shard 1](https://github.com/paperclipai/paperclip/actions/runs/39704932041/job/119582710174) failed the unchanged workspace-runtime test that replaces an unhealthy local process. The replacement refused connections for its original 10-second readiness budget. The fixture and runtime service are unchanged and do not import tool-access. One isolated run passed without changes or timeout increases. The failure log is preserved; the cause is not reproduced. Only that failed job was retried; it passed at the same head without code changes. The original failure remains recorded above. ## Risks - Moving profile mutations could change transaction clients or lock order. Both are unchanged and the existing database tests cover publication and assignment behavior. - Effective assignment resolution still uses the same installed-connection loader and full catalog projection. The lightweight path still omits large schema and metadata fields. - No schema, public API or access policy change is intended. Existing tests provide characterization coverage; no new test mirrors the extraction. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code 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> |