Commit Graph
2 Commits
Author SHA1 Message Date
DottaandPaperclip 96824a71d1 fix(db): migrate connector hooks on SQL_ASCII databases (#16958)
## Thinking Path

> - Paperclip manages work for AI agents.
> - Server startup applies database migrations before accepting work.
> - Some older embedded databases use SQL_ASCII encoding.
> - Migration 0355 uses a Unicode escape literal that PostgreSQL rejects
on those databases.
> - A source rebuild can pass but the updated server cannot start.
> - This change makes that migration portable and preserves its shipped
receipt.
> - Existing databases retain their newer connector triggers during
upgrades.

## Linked Issues or Issue Description

**What happened?**

A source deployment failed at startup with `conversion between UTF8 and
SQL_ASCII is not supported`. Migration 0355 could not compile its
connector policy function on an older embedded database. The previous
app was restored while this repair was prepared.

**Expected behavior**

Supported legacy databases complete startup migrations. Connector
selector trimming matches JavaScript `trim()`. A database that already
applied 0355 must not replay it over the later 0356 connector trigger
definition.

**Steps to reproduce**

Create a PostgreSQL database from template0 with SQL_ASCII encoding,
then apply the current migration chain. The new regression fails with
the observed conversion error before this change.

**Paperclip version or commit**

Reproduced on `92b238137ba716cdf1746dd49969e2f5ab7af8fe`. The branch
includes current master `f57ed25385`.

**Database mode**

Embedded PostgreSQL 18.1; legacy SQL_ASCII database. The UTF-8 path is
also covered.

Related background: #645 fixed encoding for new databases. Existing
SQL_ASCII installations still need portable migrations. #16956 fixes a
separate workspace lock timeout that motivated this deployment.

## What Changed

- Build the whitespace pattern from UTF-8 bytes instead of a Unicode
escape literal.
- Match whole whitespace sequences so SQL_ASCII does not trim partial
multibyte characters.
- Recognize the original 0355 checksum as a valid receipt. This prevents
replay of its older trigger body over 0356.
- Add real PostgreSQL coverage for both encodings, selector behavior,
and preservation of the later trigger definition and receipt count.
- Document the legacy encoding and migration-history contract.

## Verification

- Red: SQL_ASCII migration failed with the same conversion error; UTF-8
passed.
- Second red: fixing only the SQL caused an existing receipt to replay
and overwrite the later connector function.
- Green: 29 focused tests passed across the new regression, migration
replay tests, and database client tests.
- Rebased regression: all 29 focused tests passed.
- `pnpm -r typecheck`, `pnpm build`, and `pnpm --filter @paperclipai/db
check:migrations` passed.
- Linux ARM64: clean build and 37 focused tests passed, including both
encoding and workspace-lock regressions.
- Live acceptance: the same legacy SQL_ASCII database completed all
seven remaining migrations. The server reports the tested commit and
healthy status. The restart report has no lost runs.
- All 55 current-head checks passed; two unrelated Storybook checks were
skipped. Full test, typecheck, build, runner, release, and E2E CI gates
passed. The complete suite ran in CI; local verification used focused
tests.
- Greptile reviewed this exact commit at 5/5 with no findings.
- Preserved failures: the initial unmodified-master startup failed at
migration 0355 and was rolled back. A later build-cache path error in
the deployment commands was corrected before the successful clean build;
it required no source change.

## Risks

The migration content changes, so receipt compatibility matters. The
original checksum remains recognized to avoid replaying historical DDL.
The tests compare complete later function definitions and receipt
counts. The whitespace pattern contains encoded bytes; tests cover every
ECMAScript whitespace character and preserve non-whitespace Unicode
input. Database data and schema shape are unchanged by this repair.

## Model Used

OpenAI GPT-6 through Codex, with repository tools, code execution, and
browser verification. The exact served model variant 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>
2026-10-20 07:56:32 -05:00
Nicky LeachandPaperclip 194fc11406 feat(plugins): invalidate runtimes when tool definitions change (#16604)
## Thinking Path

> - Paperclip is the open source app people use to manage AI agents for
work.
> - Lifecycle plugins must refresh runtime preparation when tool
definitions change.
> - Standing assignments alone do not cover catalogs, profile entries,
or policies.
> - New journal hooks must preserve the order used by setup
finalization.
> - This pull request adds tool-definition invalidation and its required
writer guards.
> - It leaves credentials and provider access changes for the next
independent capability.

## Linked Issues or Issue Description

**What happened?**

Catalog, profile-entry, policy, gateway, and connection-configuration
changes do not request agent runtime synchronization. A new
profile-entry trigger also exposes a deletion/finalization deadlock if
the child row is changed before its parent is locked.

**Expected behavior**

Committed tool-definition changes advance runtime revisions without
polling. Bookkeeping does not. Entry changes and profile replacement use
a consistent parent/reference lock order and commit atomically.

**Steps to reproduce**

1. Change a catalog descriptor, profile entry, policy, or connection
configuration.
2. Expect committed runtime events for the affected agents.
3. Run profile-entry deletion concurrently with setup finalization while
both wait for an agent row.
4. Expect both operations to complete without PostgreSQL deadlock 40P01.

**Additional context**

Extracted from #16503. Catalog publication #16602 and standing
assignments #16603 are merged. This PR is rebased onto master and is the
active review. It addresses the original Apex profile-entry deletion
finding. Credential/access hooks #16605 remain a draft without review
requests.

## What Changed

- Add migration 0355 for application/connection configuration, catalog,
profile-entry, policy, and MCP gateway invalidation.
- Preserve transaction coalescing and ignore health bookkeeping and
trust-rule usage counters.
- Keep credential-only fields outside this migration's coverage.
- Preserve merged recipient scoping for bindings and installations.
Profile-entry edits resolve old/new standing profile bindings. Policy
edits resolve old/new agent selectors with their intersection/list
semantics.
- Lock old and new profile-entry references before parent profiles and
child mutations. Recheck the reference set under the parent lock, retry
changed snapshots up to three times, and merge unspecified fields from
the locked profile.
- Make entry add/edit/delete and profile replacement transactional,
including validation and profile reads on the reserved connection.
- Lock policy rows in ID order and change priorities in one statement so
scoped agent locks are ordered across the full set.
- Lock the managed profile before connection removal deletes its entries
in the same transaction.
- Update assignment tests whose old absence assertions are superseded by
expanded configuration coverage.
- Document the supported capability and the remaining credential/access
boundary.

## Verification

- Final head: `809fcb11794bfb12c545af0a2f96ab9bc2b570a2`. It is rebased
onto master with merged #16623. GitHub reports no merge conflicts. [CI
run
39293925136](https://github.com/paperclipai/paperclip/actions/runs/39293925136)
has 55 successful checks and two optional Storybook skips.
- Standard Greptile returned an independent 5/5. The final Apex review
returned 5/5 on this exact head at 17:47 UTC on 2026-10-19. Its summary
explicitly clears this PR for merge independently of draft #16605. The
security scan passed. All review threads are resolved.
- Two real-PostgreSQL negative controls reproduce `40P01` without the
old-reference guard or the concurrent-snapshot check. Both regression
tests pass with the final correction. Server typecheck, migration safety
and numbering, five-snapshot retention, journal history, feature maps,
and diff checks pass. The actual PR diff was scanned for secrets and
personal data before push.
- The final 15-suite local run passed 821/822 tests. The one failure was
in the unchanged runtime-delivery test: a startup callback changed its
global callback count during an independent transaction. The test-only
correction is isolated in #16871. That PR passes all 17 runtime-hook
tests, server types, hosted CI, and standard Greptile 5/5. Its
notification-disabled control fails at the independent event-delivery
assertion. No changes from #16871 are included here; this feature head
passes hosted CI independently.
- Earlier affected-suite runs passed 814 tests after rebase and 820
tests after recipient and whitespace corrections. Four affected suites
passed 514 tests before the second reference-contention regression was
added. Single-connection tests cover entry add, edit, delete, and
replacement. Validation and profile reads use the reserved transaction
connection.
- Review fixes have negative controls: the old code hangs with a
one-connection pool; parent-lock omissions deadlock entry deletion or
app removal against setup finalization; per-row policy reorder deadlocks
under contention; unscoped triggers journal unrelated peers; default SQL
trimming misses JavaScript whitespace. Corrected cases pass. Selector
tests cover all 25 ECMAScript whitespace characters. Recipient tests
cover repeated edits, old/new targets, ignored bindings, rollback,
coalescing, and company scope. The definition suite now has 28 cases.
- Earlier standard scores that depended on #16605 were not accepted as
independent clearance. The first Apex review returned 4/5 for
replacement versus application deletion. The final correction locks old
and new references, checks the reference set again under the parent
lock, and retries before any mutation. The final independent reviews
clear that correction.
- Preserve prior failures for acceptance: draft run 38975432017 had
failures whose causes were not re-investigated. Run 38990400765 exposed
the unrelated durable-resume watchdog race, fixed in merged #16623. In
run 39287657148, two jobs reported cancellation without a failed-test
summary; the cause remains unknown. Both passed on retry without source
changes. Initial local fixtures and SQL bindings also needed correction:
an internal helper name was replaced with public `archiveConnection`,
unchanged priorities were changed to exercise reorder, a driver
timestamp was bound as ISO text, and a superseded company-fanout
assertion was updated. These recovered checks do not establish a cause
for the unexplained CI failures.
- Local checks target the affected services and real database
boundaries. A full local build and repository-wide test run were not
performed. Hosted CI supplies the broader checks. Provider responses in
tests are controlled; live provider behavior was not qualified.

## Risks

Migration 0355 adds statement triggers without changing table schema.
Apply it through the normal migration flow. A tool-definition event can
advance a runtime revision without changing the bounded assignment
snapshot. Consumers still need authorized readers for definitions.
Credential-only changes, grants, secrets, and endpoints remain for the
access-hook migration. Profile replacement now rolls back as a unit if
any entry fails. Binding and install notifications retain scoped
recipients. Profile-entry changes use standing bindings, and policies
use old/new agent selectors. Unscoped policies, company-bound profiles,
and other conservative definition changes retain synchronous
transaction-atomic company-wide fanout: O(eligible company agents) locks
and journal storage, without a fanout ceiling or mutation quota. This
retains the accepted tradeoff from #16603 for company-wide changes.

## Model Used

OpenAI Codex, GPT-6, with reasoning, repository inspection, code
execution, and testing. The exact deployment model ID 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 local checks; results and the isolated test failure are
recorded above
- [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>
2026-10-19 10:56:08 -07:00