mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-29 04:25:52 +01:00
fix(server): schedule tool health from committed changes and deadlines (#17554)
## Thinking Path > - Paperclip manages AI-agent work and its connected tools. > - Idle hosted instances need their database and network activity to stop. > - The maintenance tick queries tool connections even when no health check is due. > - Existing connection rows already contain eligibility and the last health observation. > - This pull request reconstructs health deadlines and refreshes them after committed changes. > - The shared coordinator preserves retries, drain, and shutdown without another private timer. ## Linked Issues or Issue Description **What happened?** The aggregate maintenance tick scans connection health every 30 seconds. Empty instances and connections with fresh health observations still cause SQL activity. **Expected behavior** No eligible connection means no recurring health SQL. Eligible connections retain the existing 15-minute proactive check. Configuration changes and health observations update the schedule after commit. **Steps to reproduce** Start a server with heartbeat scheduling enabled and no tool connections. Let startup finish and observe two maintenance ticks. The old code repeats the health inventory query. The new startup regression verifies that those ticks do not invoke the health inventory. **Paperclip version or commit** This branch includes merged parent #17518 and master `96727b3cfefb61f85ffe1fbc29a9b6dd6837efe8`. The 47 changed paths belong to the single tool-connection health duty. No database file differs from master. Related: #14638 covers runtime-auth exclusions. #15448, #15325, and #11910 concern MCP failure classification. This PR preserves those provider policies and does not replace their work. ## What Changed - Keep the GitHub error-projection unit fixture at the health-write boundary and verify the real transaction path in the PostgreSQL health-worker suite. - Add a health schedule owned by tool access. Read lean eligibility fields and saved observation timestamps. - Register health work with the existing coordinator. Refresh only dirty connection/application/company scopes using bounded inventory pages, check due connections in fair company batches, continue concrete due backlog after one second, and retain failures with backoff. - Notify before supported eligibility and observation writes in their owning transactions. Cover API/gallery setup, OAuth, catalog/gateway observations, email, voice, access removal, deletion, and settings. - Commit connection creation with grant, binding, and runtime-slot SQL before its wake. Keep provider calls outside the new transactions. - Keep provider-to-transport mapping in the chat catalog. SDK-only chat changes do not wake health work. - Replace aggregate health scans, retain the public manual sweep, and document the producer audit and remaining sleep limits. - Keep metadata-only example and Smoke Lab installs scoped. Lock and re-read application type before publishing a real eligibility change. - Keep failed inventory reads on their own retry deadline while healthy due batches continue. - Apply fixture migrations before using external disposable databases in all six producer suites. ## Verification Published head: `8c1d293d1ae2b7db753ab20f5a2a2edec1fb0a62`. Fresh CI and reviews are required for this exact head. - **623 distinct targeted cases pass.** The final four complete health, fair-batching, GitHub health recovery, and startup suites pass 80 cases after the retry fix. Five complete producer suites pass 543 cases: tool access 469, access 18, email 45, GitHub events four, and Smoke Lab seven. The earlier 26 health cases are part of this total, not additional cases. - The six producer suites first passed against separate, initially empty PostgreSQL 17 databases at `acbf262843`. They apply their own migrations. The subsequent repair changes only the health worker and its regression; its complete final consumer suites pass on that integrated parent. Server typecheck and build pass with the final repair. - **Installer amplification:** five real installs with 205 unrelated connections reproduced 1,035 example rows and 1,045 Smoke Lab rows. Repeated metadata installs now load only changed fixture connections. The same cases restore an excluded application through each installer and require all 205 unrelated IDs to re-enter the schedule. - **Concurrent eligibility:** a real PostgreSQL lock-wait case requires the installer to notify after a concurrent type change. A mutation that retains the lock but uses the old lookup fails the settled-hint assertion; original source was restored byte for byte. - **Retry isolation:** the failing baseline read one unavailable company three times while another company's 60 due checks drained. Final tests require one initial read, at least 30 seconds between failed reads, prompt healthy progress, eventual recovery, and SQL quiet after settlement. - Real-service producer coverage includes AgentMail activation/pause, GitHub installation changes, and member access removal. Lifecycle coverage includes outer commit/rollback, startup reconstruction, drain/resume, failure, and shutdown. - Failure history and exact evidence scope: [health acceptance](https://github.com/paperclipai/paperclip/blob/8c1d293d1ae2b7db753ab20f5a2a2edec1fb0a62/doc/idle-tool-connection-health.md#acceptance-follow-up-after-the-plugin-parent-merged-october-27), [producer audit](https://github.com/paperclipai/paperclip/blob/8c1d293d1ae2b7db753ab20f5a2a2edec1fb0a62/doc/plans/2026-10-24-tool-connection-health-sleep-audit.md). Prior remote failures and earlier 706-case evidence remain historical. Startup tests emitted listener-count warnings; all assertions passed. No full local repository-suite, deployed sleep, fleet savings, or live provider qualification claim is made. ## Risks - Supported live writers must use the transaction signals. Independent SQL writers and other replicas need their own wake integration. - Configured eligible connections still have a real proactive health obligation. This does not prove Cloud alarm coverage or host/database suspension. - Existing manual probes and catalog operations can overlap. This migration does not introduce a credential-version CAS or change provider failure policy. - Voice remains eligible under the previous health policy. Its generic probe can still report an unsupported transport; changing that behavior is separate work. - No database migration, new table, workflow, or lockfile change. This PR includes its merged parent and current inspected master; later stack PRs and all current-head acceptance gates remain separate. ## Model Used OpenAI Codex, GPT-6 family, with repository tools, code execution, and reasoning. The exact serving version 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 - [ ] 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 --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
5cfa2bf26d
commit
94a780286f
47 files changed
+1795
-246
No files matched your search
@@ -0,0 +1,291 @@
|
||||
# Tool connection health while idle
|
||||
|
||||
The health worker replaces the aggregate maintenance tick's repeated connection
|
||||
inventory query. It preserves the existing 15-minute proactive health interval
|
||||
for eligible connections. This is a deadline migration, not removal of proactive
|
||||
health checks or proof of host suspension.
|
||||
|
||||
## Ownership and lifetime
|
||||
|
||||
`tool-access/catalog/health-schedule.ts` owns eligibility and saved deadlines.
|
||||
The existing health and catalog services still own provider behavior. The worker
|
||||
uses the shared delivery coordinator for admission, wake coalescing, retry,
|
||||
drain/resume, and shutdown. Startup reconstructs eligible deadlines from existing
|
||||
connection rows. No new table, polling listener, or private interval is added.
|
||||
|
||||
Eligible rows are active and enabled, excluding `chat_sdk`, `runtime_auth`, and
|
||||
applications of type `paperclip_plugin`. Disabled experimental connectors have
|
||||
no deadline until the settings producer enables them. Application status and
|
||||
company status are not new filters: this migration preserves the old policy.
|
||||
|
||||
An unchecked row is due immediately. Otherwise its deadline is `healthCheckedAt`
|
||||
plus 15 minutes. Individual writes carry a connection ID and reload only that
|
||||
connection. Application mutations refresh only that application's connections;
|
||||
company deletion invalidates the company's cache. Startup and instance feature
|
||||
settings can refresh the whole instance. Inventory reads use UUID keyset pages
|
||||
of at most 100 rows and observe shutdown cancellation between pages. Refreshes are
|
||||
coalesced with a 30-second minimum spacing when no due backlog remains.
|
||||
Batches retain at most 25 checks and continue after one second while already due
|
||||
rows remain, so the batch limit does not cap dispatch at 50 checks per minute. There is no scheduled refresh when
|
||||
there are no eligible rows or unresolved changes. Future checks cause no SQL
|
||||
before their deadline. Each pass checks at most 25 due connections, taking turns
|
||||
across companies and preserving due order within each company. The company cursor
|
||||
rotates across passes when more companies are due than fit in one batch.
|
||||
|
||||
Before a provider check, the service reloads current eligibility and deadline.
|
||||
A saved health result, including a failed health observation, owns the next
|
||||
15-minute deadline. A check that cannot persist an observation retains recovery
|
||||
debt with exponential backoff from 30 seconds to 15 minutes. A failed scoped
|
||||
refresh does not prevent checks outside that connection, application, or company. Dirty scopes arriving during
|
||||
a refresh remain pending. Drain retains them without passive database reads.
|
||||
Shutdown joins admitted work; it does not abandon a provider call midway.
|
||||
|
||||
## Producer coverage
|
||||
|
||||
| Owner | Scheduling change |
|
||||
| --- | --- |
|
||||
| Tool access API and examples | Create, enable/disable, reconfigure, revive, or restore a connection; setup grants, credential bindings, and runtime slots commit before a creation wake |
|
||||
| Gallery setup and failure handling | Activation, temporary quarantine, failed-identity restoration, and failed setup deletion |
|
||||
| OAuth callbacks and finalization | Direct, Cloud, Vercel, and personal/shared activation; reauthorization-required disablement |
|
||||
| Connection removal | Archive and disable in the owning transaction |
|
||||
| Health/catalog publication | Successful or failed saved health observation moves the deadline |
|
||||
| Gateway actual use | Saved remote connection health moves the deadline |
|
||||
| GitHub installation events | Health observation commits with the installation-event transaction |
|
||||
| Email setup/control/health | REST connection activation, pause/resume/removal, successful receipt health, and failed health observations |
|
||||
| Shared chat lifecycle | Voice health activation, setup, pause/resume/removal, resource reconciliation, inbound activation, and failure observations; SDK-only chat mutations do not wake this worker |
|
||||
| Access removal | Losing the last applicable credential grant disables the connection |
|
||||
| Smoke Lab | Connection setup and fixture health updates |
|
||||
| Company deletion | Removes cached deadlines after cascading deletion commits |
|
||||
| Instance settings | Experimental connector eligibility changes use an administrative all-company hint |
|
||||
|
||||
Notifications are awaited before the relevant SQL write. Existing transactions
|
||||
remain the owner; standalone mutations use the same small transaction helper.
|
||||
Nested calls retain the shared database client's outer-commit, rollback, and
|
||||
uncertain-outcome protocol. Provider requests are outside newly added
|
||||
transactions. The health worker never interprets provider protocols.
|
||||
|
||||
Credential-only refresh, refresh lease acquisition/release, client registration,
|
||||
OAuth authorization-start metadata, legacy credential repair, Slack-search OAuth,
|
||||
aggregator account metadata, and install markers do not change health eligibility
|
||||
or `healthCheckedAt`. They do not gain health notifications. A due probe reads
|
||||
current credentials. AI writers use the excluded runtime-auth transport.
|
||||
Application display/status updates do not change the old health eligibility.
|
||||
Application deletion refuses linked connections; company cascade is covered.
|
||||
New draft-only connections remain ineligible until their activation producer.
|
||||
|
||||
## Limits
|
||||
|
||||
The supported live writer model is this server's instrumented services. Offline
|
||||
restore is reconstructed at startup. Independent SQL writers and other replicas
|
||||
need their own wake integration; process-local hints do not cover them.
|
||||
|
||||
Existing manual health checks and catalog operations can overlap. This migration
|
||||
does not introduce a credential-version CAS or change provider failure policy.
|
||||
The scheduled worker itself is non-overlapping and rechecks the current row before
|
||||
probing. A naive `updatedAt` CAS would reject a health check's own catalog writes.
|
||||
Voice remains eligible under the old sweep's policy even though its generic
|
||||
health probe can report an unsupported transport. Changing that policy belongs
|
||||
to the voice owner, separately from this scheduling migration.
|
||||
|
||||
The Cloud sleep-safety proof is unchanged. An in-process deadline cannot wake
|
||||
stopped compute; unsupported external wake obligations remain blockers.
|
||||
|
||||
## Verification in progress
|
||||
|
||||
The original startup regression showed repeated idle health sweeps. It now proves
|
||||
one scheduled-worker reconstruction and no health inventory work from aggregate
|
||||
ticks. Seventeen real-PostgreSQL lifecycle tests pass: empty/configured quiet,
|
||||
excluded transports, first writes, nested commit/rollback, drain/resume, saved
|
||||
observations, restart, page progress, real API activation/disable/archive,
|
||||
company failure isolation, feature toggles, company deletion, voice activation,
|
||||
concurrent producer writes, failed persistence, and admitted shutdown.
|
||||
|
||||
The first consumer run passed 514 cases and failed a rollback fault fixture tied
|
||||
to transaction call order. The fixture now rejects the actual restoration update
|
||||
with a PostgreSQL trigger. All 515 startup/tool-access cases pass afterward.
|
||||
Another 163 gateway/settings cases pass. Server typechecking passes after fixing
|
||||
two mutable values captured by transaction callbacks. Additional producer/chat
|
||||
consumers, query-plan inspection, build, and remote review/checks remain pending.
|
||||
|
||||
An initial additional-consumer command ran before its external-database fixture
|
||||
patch applied: embedded PostgreSQL failed to start and 72 tests were skipped.
|
||||
That run is not passing evidence. The corrected run uses validated disposable
|
||||
PostgreSQL databases; results will be recorded before PR acceptance.
|
||||
|
||||
The corrected producer run first passed all 44 email cases but contaminated the
|
||||
other suites with shared fixture rows, causing foreign-key cleanup failures.
|
||||
Separate freshly initialized databases preserve each suite's original isolation;
|
||||
all 72 access/email/GitHub/smoke-lab cases then pass. Three selected chat credential
|
||||
and pause cases pass; 1,414 deselected cases are not counted as passing coverage.
|
||||
The producer mapping/voice wake contract is covered by the health lifecycle test.
|
||||
Server build passes. The scoped inventory SQL with 40,000 archived and 10 active
|
||||
rows uses `tool_connections_company_enabled_idx`, 13 shared buffer hits, and
|
||||
0.022 ms execution; the diagnostic data was rolled back. No new index is needed
|
||||
for this observed query. The local passing selection totals 770 distinct tests.
|
||||
Full workspace and remote acceptance gates remain required.
|
||||
|
||||
Review repair: a controlled 503 probe persisted its failed health observation,
|
||||
but the warning regression failed before the fix. Scheduled probe failures now
|
||||
log company/connection identifiers and a fixed message, without provider prose.
|
||||
The saved 15-minute deadline still controls the next check. All 64 health/startup
|
||||
cases and server typechecking pass after repair; total selected coverage is now
|
||||
771 unique tests. Fresh remote gates are required for this repair.
|
||||
|
||||
|
||||
Review follow-up: fair round-robin company batching replaces the global oldest-25
|
||||
selection. The two-company regression failed before the repair; 19 health-worker
|
||||
and three shared batching cases pass afterward. Each probe's pre/post eligibility
|
||||
query already includes the connection ID, so those indexed current-state checks
|
||||
remain. A scheduled observation still emits the normal transaction signal and
|
||||
now refreshes only that connection following the security repair below. This
|
||||
preserves uncertainty and concurrent-writer recovery without a silent-write exception.
|
||||
|
||||
The real-service producer suite now proves AgentMail activation triggers its first
|
||||
health probe, pause cancels the next deadline, GitHub installation events update
|
||||
the cached deadline, and member removal cancels a personal connection's deadline.
|
||||
The GitHub newer-access branch retains its old health deadline, as intended.
|
||||
These tests use the actual coordinator, transaction notifications, schedule query,
|
||||
and service mutations; AgentMail also runs the actual health service against its
|
||||
controlled provider. Subsequent idle windows assert no SQL.
|
||||
|
||||
The first run exposed two incorrect fixture expectations: AgentMail setup leaves
|
||||
healthCheckedAt null until its first probe, and a stale GitHub installation event
|
||||
does not overwrite a newer access refresh. The tests now preserve those policies.
|
||||
A targeted rerun reused AgentMail's retained fixture user and failed setup with a
|
||||
duplicate key; a fresh disposable database fixes that isolation issue without
|
||||
changing application behavior or weakening assertions. All 66 access/email/GitHub
|
||||
cases pass on the final run. Server typecheck/build pass. Cumulative selected
|
||||
coverage is 776 unique tests; full-suite and fresh remote review/checks remain
|
||||
acceptance gates. No whole-instance sleep or provider suspension is claimed.
|
||||
|
||||
|
||||
The Discord child review also exposed fixed-batch throughput as a risk for health.
|
||||
A probe-timestamp regression measured a 30.7-second gap before the 26th already
|
||||
owed check. The worker now schedules a one-second continuation only when the due
|
||||
selection exceeds its bounded batch. Failed work retains backoff; an empty or
|
||||
fully future-dated inventory has no continuation. This is scheduling concrete
|
||||
backlog, not a periodic idle scan. The initial deadline-only assertion passed
|
||||
incorrectly because a pending transaction notification temporarily reported an
|
||||
immediate coordinator wake; acceptance measures actual probe timestamps instead.
|
||||
|
||||
|
||||
## Security review: bounded refresh scope
|
||||
|
||||
Security review identified amplification in the original company hints. Five
|
||||
separate real `updateConnection` calls in a company with 205 eligible connections
|
||||
returned 1,025 inventory rows. A regression reproduced that result. The repaired
|
||||
producer contract returns exactly five rows across those updates. Saved probe
|
||||
observations use the same single-connection scope, so draining a due backlog no
|
||||
longer reloads its whole company after each batch.
|
||||
|
||||
The typed hint accepts an existing connection row (only company and connection
|
||||
IDs are encoded), or an explicit application target. Company/global scopes remain
|
||||
for company deletion and administrative feature changes. Smoke Lab allocates a
|
||||
new connection ID before its insert so the transaction intent is registered before
|
||||
the write. No schema, outer transaction ownership, or provider policy changes.
|
||||
Application eligibility changes retain their own scoped refresh. Single-connection
|
||||
refreshes remove an old cached deadline even when the row was deleted, disabled,
|
||||
or moved to an excluded application; unrelated deadlines remain intact.
|
||||
|
||||
Inventory queries use 100-row UUID keyset pages. A 205-row regression initially
|
||||
observed one unbounded query and now requires three pages with all 205 distinct
|
||||
IDs retained. Cancellation is checked between pages. Results replace a scope only
|
||||
after its complete load succeeds; failed loads retain recovery and other scopes
|
||||
can proceed. Malformed hint shapes request full reconstruction instead of dropping
|
||||
later scopes.
|
||||
|
||||
The first amplification fixture lacked its required remote endpoint and failed
|
||||
validation before reaching its assertion. After adding a controlled endpoint
|
||||
lookup, the baseline reproduced 1,025 versus five. The first typecheck found a
|
||||
partial access-service projection without companyId; the target now supplies the
|
||||
already-owned companyId explicitly. These failures are retained as evidence, not
|
||||
counted as passes. Final verification and remote review are recorded below.
|
||||
|
||||
|
||||
Final scoped-refresh verification: 26 health/batching cases, 63 email/access
|
||||
cases, 56 GitHub/Smoke Lab/startup cases, 469 tool-access cases, and 92 gateway
|
||||
cases pass (706 targeted tests on this repair). Server typecheck and build pass.
|
||||
The full local test matrix was not rerun; prior remote Actions passed at
|
||||
f8ad26f0ff, which still had the security finding. Fresh remote checks/security
|
||||
review on the repaired head are required and are not inferred from those results.
|
||||
|
||||
## GitHub health unit fixture after master integration (October 27)
|
||||
|
||||
Fresh CI on #17649 failed server shard 12 in the new GitHub suspended-installation
|
||||
recovery test. Its minimal database fake only implemented reads and updates;
|
||||
health changes now go through the existing transactional write owner. The test
|
||||
therefore received `db.transaction is not a function` instead of its expected
|
||||
422 response and installation settings links. The same failure reproduced on
|
||||
this owning PR before correction.
|
||||
|
||||
The unit test now substitutes the health-write collaborator while preserving
|
||||
all of its original error, message, connection-health, and URL assertions. This
|
||||
is the boundary of that error-projection fixture, not a change to production
|
||||
transactions or a substitute for transactional acceptance. The separate complete
|
||||
`tool-connection-health-work` suite still uses the real write owner and a fresh
|
||||
PostgreSQL database. Both complete files pass all 24 cases with no skips on owner
|
||||
source based on `d2068f59a30f6f33b4543a79b539016294cbfe15`.
|
||||
|
||||
The failed CI job is
|
||||
[server shard 12](https://github.com/paperclipai/paperclip/actions/runs/40470614670/job/122077293740).
|
||||
Fresh remote CI is still required on the follow-up head. Other review findings
|
||||
on health scheduling remain separate; this fixture correction does not resolve
|
||||
them or claim deployed sleep.
|
||||
|
||||
## Acceptance follow-up after the plugin parent merged (October 27)
|
||||
|
||||
PR #17518 is merged as `96727b3cfefb61f85ffe1fbc29a9b6dd6837efe8`.
|
||||
The health branch includes that actual master through a normal merge. Its 47
|
||||
health-owned paths are the only differences from that master; it changes no
|
||||
database files. All 18 preceding sleep PRs are merged. Older statements above
|
||||
about an unmerged stack and prior acceptance describe their recorded revisions.
|
||||
|
||||
A second security finding exposed a remaining application-wide invalidation in
|
||||
example installation and Smoke Lab. With 205 unrelated eligible connections
|
||||
under an installed application, five actual repeated installs loaded 1,035 rows
|
||||
for the example and 1,045 rows for Smoke Lab. These are failing row-count
|
||||
regressions, separate from the earlier individual-connection amplification.
|
||||
Metadata-only upserts now retain connection-scoped hints from the actual fixture
|
||||
writes. Application-wide refresh occurs only when its type changes eligibility.
|
||||
The same tests restore an excluded application through each real installer and
|
||||
require all 205 unrelated connection IDs to return to the live schedule.
|
||||
|
||||
The upsert reads the current type under its owning transaction's PostgreSQL
|
||||
`FOR NO KEY UPDATE` lock. This matches the non-key update and permits foreign-key
|
||||
key-share locks. An earlier lookup cannot establish the current type after a
|
||||
concurrent writer commits. A real lock-wait test confirms that the installer
|
||||
notifies the restored application after the other writer releases its lock.
|
||||
Replacing that current-type comparison with the earlier lookup reproduces a
|
||||
missing settled hint; the original source is restored after that mutation check.
|
||||
No provider call moves inside this transaction.
|
||||
|
||||
The latest Greptile finding also reproduced: while 60 healthy due connections
|
||||
drained in three bounded batches, a different company's failing inventory read
|
||||
ran three times instead of once. Failed inventory scopes now retain their own
|
||||
30-second retry deadline. Healthy backlog continuations cannot shorten it, and
|
||||
cached connections in an unavailable scope cannot be probed from stale data.
|
||||
The regression requires all healthy checks to continue promptly, failed reads
|
||||
to remain at least 30 seconds apart, eventual recovery, and SQL quiet after
|
||||
settlement. Successful global reconstruction clears the covered failed scopes.
|
||||
This changes only the health owner; coordinator and database contracts stay intact.
|
||||
|
||||
The six external-database producer suites now apply pending fixture migrations
|
||||
before creating their client. All six pass against separate, initially empty
|
||||
PostgreSQL 17 databases on source `acbf262843`: 26 health, 469 tool-access,
|
||||
18 access, 45 email, four GitHub-event, and seven Smoke Lab cases (569 total).
|
||||
Server typecheck and build also pass on that source. The subsequent health
|
||||
retry repair has a separate failing baseline and passing focused test; its
|
||||
complete consumer and type/build verification is recorded with publication.
|
||||
These source checks do not establish deployed suspension, Cloud wake coverage,
|
||||
a full local repository-suite pass, or savings. Fresh current-head reviews and
|
||||
all CI gates remain mandatory before merge.
|
||||
|
||||
Final retry-repair acceptance: all four complete health, company-batching,
|
||||
GitHub health recovery, and server startup suites pass 80 cases. Together with
|
||||
the five complete producer suites above, this covers 623 distinct cases; the
|
||||
26 earlier health cases are included in that total rather than added again.
|
||||
Server typecheck and build pass with the retry repair. Startup tests emit the
|
||||
existing SIGINT/SIGTERM listener-count warnings; no assertion failed. The fresh
|
||||
fixture results and these final worker results have different source scopes as
|
||||
stated above. The failed amplification, stale-type mutation, and retry baselines
|
||||
are retained; a passing acceptance does not erase those failures.
|
||||
@@ -1291,3 +1291,101 @@ and integrates that accepted parent. Normal ancestry reconciliation preserves
|
||||
its entire verified tree (`df78ca3b5e`) byte for byte; only publication docs change
|
||||
subsequently. All original remaining duties remain in scope, beyond the 65
|
||||
published numbered slices, including native backups and remaining aggregate owners.
|
||||
|
||||
## Slice 18: tool-connection health implementation
|
||||
|
||||
The health migration now reconstructs per-connection deadlines and reacts to
|
||||
committed company changes instead of inventory scans on every maintenance tick.
|
||||
See [health lifecycle and producer coverage](../idle-tool-connection-health.md).
|
||||
Existing proactive health, provider policy, voice eligibility, transaction
|
||||
uncertainty, drain, and shutdown behavior remain explicit. Seventeen lifecycle,
|
||||
515 startup/tool-access, and 163 gateway/settings tests pass. Additional producer
|
||||
consumers and publication gates remain open. This is not a completed PR yet.
|
||||
|
||||
Fresh remote inspection: approval queue #17510 at `0d3100665d` has no failed or
|
||||
pending checks. Plugin #17518 at `ba0b6752cd` has no failed checks and one pending
|
||||
Paperclip Review check. The full original inventory and #17490 E2E cancellation
|
||||
failure remain required; no PR was merged.
|
||||
|
||||
Health consumer follow-up: 72 access/email/GitHub/smoke-lab cases pass using a
|
||||
separate disposable DB per suite, and three selected chat cases pass. An initial
|
||||
shared-DB attempt produced cleanup foreign-key failures after email; those are
|
||||
recorded, not counted as passes. Total selected coverage is 770 cases. The server
|
||||
build passes. The inventory SQL uses the existing company/enabled index with
|
||||
40,000 retained rows (13 buffers, 0.022 ms). Full workspace and remote gates remain.
|
||||
|
||||
|
||||
## Discord publication and health fairness repair
|
||||
|
||||
Discord registration is [#17557](https://github.com/paperclipai/paperclip/pull/17557),
|
||||
head `3755d7caced67589de474e0ca3e1b8b5b4e659c4`, based on #17554 and attached.
|
||||
It has 639 selected passing tests, workspace typecheck/build, and final server
|
||||
verification. CI run **40168371442** and review run **40168371311** are confirmed
|
||||
live on that head. Fresh CI and review remain required; no merge is authorized.
|
||||
The source and detailed producer audit are on the Discord child branch.
|
||||
|
||||
The health parent Paperclip Review completed neutral with 4/5 at `da40899606`.
|
||||
It requests fair company batching and direct real-service producer coverage.
|
||||
The existing probes are already scoped by connection ID, so the review's concern
|
||||
about full inventory scans around each probe needs clarification rather than
|
||||
removing current-state eligibility checks. Self-generated observation hints still
|
||||
cause a bounded coalesced company refresh; avoid muting correctness signals merely
|
||||
to reduce that bounded work without proving equivalent recovery.
|
||||
|
||||
Fairness was reproduced with 30 unchecked connections in one company and one due
|
||||
connection in another. The quiet company was omitted from the first batch. The
|
||||
shared selector from Discord commit `e77979d4fc` was applied to the health branch
|
||||
as `097f5c8aff`; the health worker now rotates companies. All 22 health/selection
|
||||
tests pass (`/tmp/sleep18-fairness-after.log`), after the recorded failing baseline
|
||||
(`/tmp/sleep18-fairness-before.log`). The fix is local until the additional
|
||||
AgentMail activation/pause, GitHub installation event, and access-removal producer
|
||||
tests are complete. Then push the owning PR, request fresh review, and merge it
|
||||
into the Discord branch. Full original inventory and earlier unresolved CI remain.
|
||||
|
||||
|
||||
Health producer review coverage is now implemented and verified. The actual
|
||||
AgentMail setup/health/pause, GitHub installation-event, and member-removal paths
|
||||
move or cancel the live coordinator deadline. All 66 owner-suite cases pass on
|
||||
isolated PostgreSQL databases, alongside the prior 22 fairness/health cases and
|
||||
fresh server typecheck/build. Initial fixture expectations and the retained-user
|
||||
DB setup failure are preserved in the health document. Publish these changes with
|
||||
the fair company selector, request fresh review, and propagate to Discord #17557.
|
||||
The Discord initial head has Greptile 5/5; its CI and Paperclip review were still
|
||||
running when inspected. The remaining original inventory and #17490 failure
|
||||
remain open; no stack PR is merged or declared fully accepted.
|
||||
|
||||
|
||||
Discord's first Paperclip review (4/5) identified a dispatch-capacity issue shared
|
||||
by health: 25 checks per 30 seconds delays a large already-due backlog. Health's
|
||||
actual-probe timestamp regression failed with a 30.7-second gap, then passed when
|
||||
only a remaining due batch receives a one-second continuation. All 22 health and
|
||||
batching cases pass; server types pass. Propagate the parent and apply the same
|
||||
bounded continuation to Discord, with its own regression. The Discord review also
|
||||
requests preserving queued scopes after malformed hints and broader producer
|
||||
coverage. Its initial head has Greptile 5/5 but these findings remain open.
|
||||
|
||||
|
||||
Health security repair: five real connection API updates in a 205-connection
|
||||
company reproduced 1,025 inventory rows. Individual writers now signal their
|
||||
connection IDs; application writers signal their application IDs. Genuine broad
|
||||
loads use 100-row UUID pages. The regression now reads exactly five rows, with
|
||||
application invalidation and unrelated-deadline preservation verified. All 706
|
||||
targeted health/producer/consumer cases and server typecheck/build pass. Publish
|
||||
the repair to #17554, propagate through company-deletion #17561 into Discord
|
||||
#17557, and require fresh remote gates. No PR in this stack is merged. The next
|
||||
new owner remains chat runtime discovery; the full remaining ledger is unchanged.
|
||||
|
||||
## Acceptance follow-up after the plugin parent merged (October 27)
|
||||
|
||||
Eighteen preceding sleep PRs are merged through plugin scheduling #17518 at
|
||||
`96727b3cfefb61f85ffe1fbc29a9b6dd6837efe8`. Health #17554 now includes that
|
||||
actual master. It remains the next unmerged dependency, followed by company
|
||||
chat deletion #17561 and Discord registration #17557. Later children cannot be
|
||||
merged into their feature parents to bypass this order.
|
||||
|
||||
See [health acceptance](../idle-tool-connection-health.md#acceptance-follow-up-after-the-plugin-parent-merged-october-27)
|
||||
for the remaining installer amplification and independent retry repairs, failing
|
||||
baselines, real concurrency proof, and fresh-database verification. Older stack
|
||||
status and test counts above are historical. The original remaining inventory
|
||||
is still in scope beyond the 65 published numbered slices; native backups and
|
||||
the remaining aggregate duties have not been migrated by this follow-up.
|
||||
@@ -0,0 +1,196 @@
|
||||
# Tool connection health sleep migration audit
|
||||
|
||||
Parent: plugin scheduling #17518, local head `faac1bf85c`.
|
||||
Fresh master inspected during this continuation: `71f06d84dc`.
|
||||
Branch: `codex/sleep-18-tool-connection-health`.
|
||||
This is an in-progress audit and failing baseline, not a completed migration.
|
||||
|
||||
## Obligation and observed blocker
|
||||
|
||||
The aggregate heartbeat calls `tools.sweepConnectionHealth()` at startup and on
|
||||
its recurring maintenance tick. The health owner selects due connection IDs even
|
||||
when no eligible connections exist. Its default freshness threshold is fifteen
|
||||
minutes and its page size is 25. The startup regression on the unchanged worker
|
||||
expected one startup call after two maintenance ticks and observed three calls.
|
||||
The recorded command is:
|
||||
|
||||
```sh
|
||||
pnpm --filter @paperclipai/server exec vitest run \
|
||||
src/__tests__/server-startup-feedback-export.test.ts \
|
||||
-t 'does not rescan idle tool health' --maxWorkers=1
|
||||
```
|
||||
|
||||
The baseline failed with `expected 3 to be 1`. This proves aggregate activation;
|
||||
real database worker tests must separately prove quiet SQL, deadlines, and producer
|
||||
reactivation after implementation. A removed startup callback alone is not proof.
|
||||
|
||||
## Ownership and policy
|
||||
|
||||
`tool-access/catalog/health.ts` owns provider health policy and delegates actual
|
||||
provider behavior through existing discovery, credential, catalog, and runtime
|
||||
owners. Preserve this boundary. Shared scheduling must not branch on provider
|
||||
names or decode their credential payloads.
|
||||
|
||||
Existing `tool_connections.health_checked_at` and connection eligibility are the
|
||||
persistent source of truth. No additional health queue table is needed. Keep the
|
||||
manual `checkHealth` API and normal use/refresh health updates working. Preserve
|
||||
the current proactive fifteen-minute policy for eligible external checks unless
|
||||
a provider's existing event/replay contract makes a check unnecessary. Clearly
|
||||
name that retained external obligation: scheduling the next real check removes
|
||||
empty scans but does not make a configured external source network-quiet forever.
|
||||
Do not extend Cloud alarm coverage or remove active-connection sleep blockers in
|
||||
this slice.
|
||||
|
||||
The current SQL excludes `chat_sdk`, `runtime_auth`, and applications of type
|
||||
`paperclip_plugin`. The schema couples AI purpose to `runtime_auth`, and
|
||||
`checkConnectionHealth` returns immediately for AI purpose. Do not claim AI rows
|
||||
are an uncovered polling case without evidence of supported legacy rows outside
|
||||
those constraints. PR #14638 separately covers AI health behavior.
|
||||
|
||||
Audit voice eligibility, experimental connector disablement, and exceptions before
|
||||
the existing health update: a skipped check that leaves its timestamp unchanged
|
||||
must not starve later candidates or cause an immediate retry loop. Keep admission
|
||||
and authorization failures with their owners. Do not silently disable or delete
|
||||
connections to manufacture quiet.
|
||||
|
||||
## Producer inventory to finish
|
||||
|
||||
This list identifies writer families; it does not yet certify complete coverage.
|
||||
Classify each write by changes to eligibility, saved deadline, or relevant probe
|
||||
configuration before adding a notification. Excluded transport writers need no
|
||||
health wake merely because they share a table.
|
||||
|
||||
- `tool-access.ts`: direct create/update, examples, gallery install/finalization,
|
||||
legacy imports, application replacement/deletion, and credential configuration.
|
||||
- `tool-access/connections/installs.ts` and `removal.ts`: enablement, removal,
|
||||
restore paths, existing transaction ownership and downstream cleanup.
|
||||
- `tool-access/catalog/health.ts`, `publication.ts`, `discovery.ts`, and
|
||||
`aggregator-accounts.ts`: successful and failed probe/catalog results and
|
||||
eligibility changes. Health writes must move or cancel the recorded deadline.
|
||||
- `tool-access/oauth/*`: access finalization, authorization start, direct/cloud/
|
||||
Vercel callbacks, client registration, grant/connection refresh, and refresh
|
||||
lease bookkeeping. Lease-only writes are not automatically health producers.
|
||||
- `tool-access/token-broker/service.ts`, `tool-gateway.ts`,
|
||||
`tool-oauth-legacy-backfill.ts`, and `github-connection-events.ts`: actual-use
|
||||
health observations, credential repair, and access changes.
|
||||
- Email connections/channels and Slack-search OAuth: determine which existing
|
||||
REST/MCP connection writes remain eligible for this health owner.
|
||||
- Access provisioning, agent routes, smoke-lab setup, AI connection services,
|
||||
and chat provider modules: classify supported imports and shared entry points;
|
||||
do not attach unnecessary wakes to excluded AI/chat transports.
|
||||
- Application type changes and company deletion: retain SQL join eligibility and
|
||||
remove stale deadlines after cascades. Instance experimental settings are an
|
||||
intentional administrative scope, distinct from ordinary tenant mutations.
|
||||
|
||||
Use existing transaction notifications before writes, including caller-owned
|
||||
transactions. Ordinary company mutations must not activate cross-company scans.
|
||||
Startup reconstruction is a separate instance-owned obligation. Process-local
|
||||
hints are not a multi-replica or independent-SQL-writer protocol.
|
||||
|
||||
## Required verification
|
||||
|
||||
- Empty, disabled, archived, excluded-transport, and retained-history quietness.
|
||||
- Configured eligible future check: no intervening SQL; run at the saved deadline.
|
||||
- First creation/enablement and changed credentials through real service producers.
|
||||
- Successful/failed manual probe and catalog/use health updates reschedule correctly.
|
||||
- Disablement, deletion, and feature changes cancel or restore the proper obligation.
|
||||
- Page fairness with more than 25 due rows and with an early uncheckable candidate.
|
||||
- Company scope, outer commit/rollback, a commit during final reconciliation,
|
||||
drain/resume, startup reconstruction, admitted shutdown, and failure recovery.
|
||||
- Keep provider calls outside transactions and avoid concurrent checks that can
|
||||
overwrite newer credentials or health observations with an old result.
|
||||
- Actual query plan on representative retained history, current consumers,
|
||||
required build/type gates, and fresh PR review/checks.
|
||||
|
||||
## Related work
|
||||
|
||||
Open PRs #15448, #15325, and #11910 concern MCP failure classification and continued
|
||||
usability after transient tool errors. This migration must preserve that owning
|
||||
policy rather than choose new disablement behavior in the scheduler. The search
|
||||
also found #14638 for AI health exclusions. Inspect exact diffs before changing
|
||||
those paths; titles alone do not establish duplication or compatibility.
|
||||
|
||||
|
||||
## Continuation after parent review repairs
|
||||
|
||||
The health branch now includes approval backoff/shared matching at `0d3100665d`
|
||||
and plugin coalescing/shutdown repair at `ba0b6752cd`. The combined parent
|
||||
regressions pass all 73 cases. Fresh master fetched and inspected is `a0e5166e97`;
|
||||
its aggregate tick still calls the health sweep. Parent PR checks are still in
|
||||
progress. The health migration is not implemented or published yet.
|
||||
|
||||
Additional owner evidence changes the implementation plan:
|
||||
|
||||
- `checkConnectionHealth` can call catalog discovery before writing its final
|
||||
health observation. `catalog/publication.ts` itself changes health timestamps,
|
||||
config, transport config, and `updatedAt`. A blanket comparison with the
|
||||
connection's original `updatedAt` would reject this operation's own writes.
|
||||
Concurrency fencing must distinguish a newer external configuration/observation
|
||||
from the current probe's owned catalog publication; do not add a naive row-version
|
||||
check that silently drops legitimate results.
|
||||
- `oauth/refresh-leases.ts` changes config and `updatedAt` merely to acquire or
|
||||
release its existing refresh lease. Those are active-operation bookkeeping,
|
||||
not independent reasons for proactive health work. Do not notify health for
|
||||
every config-column write by pattern matching.
|
||||
- `oauth/connection-refresh.ts` has two different health-relevant transitions:
|
||||
reauthorization failure changes status to draft and disables the connection;
|
||||
successful token refresh changes credential refs/config while keeping the
|
||||
connection usable. The former must cancel eligibility; the latter must refresh
|
||||
the owning schedule without a global scan or a credential-refresh feedback loop.
|
||||
- Experimental setup policy resolves application identity and may reject before
|
||||
the current health try/catch writes a timestamp. Disabled candidates therefore
|
||||
need an explicit parked state and reactivation from instance feature changes;
|
||||
they must not monopolize the bounded due page or lose their configuration.
|
||||
|
||||
These are audited constraints, not implemented guarantees. Complete producer
|
||||
classification and prove these paths before removing the health backstop.
|
||||
|
||||
## Implemented producer contract, October 25
|
||||
|
||||
The implementation and producer dispositions are now recorded in
|
||||
[the health lifecycle document](../idle-tool-connection-health.md). Health
|
||||
creation/setup mutations commit their grant/binding/runtime-slot SQL before the
|
||||
wake. Direct/cloud/Vercel OAuth activation and refresh disablement, catalog and
|
||||
actual-use observations, email control, access removal, company deletion, and
|
||||
feature changes now notify the owner. Voice connections remain covered through
|
||||
the chat lifecycle; the provider-to-transport mapping stays in the chat catalog,
|
||||
and excluded SDK chat updates do not notify health work.
|
||||
|
||||
The aggregate health scan is replaced by the shared worker. Seventeen lifecycle,
|
||||
515 startup/tool-access, and 163 gateway/settings cases pass. The first consumer
|
||||
run exposed an ordinal transaction mock; a real SQL restoration fault preserves
|
||||
the intended regression and passes. Producer/chat consumers, query-plan evidence,
|
||||
build, and remote gates remain pending. Master was refreshed to `8415f4a679`;
|
||||
this uncommitted health branch remains based on the open migration stack, so
|
||||
final stack reconciliation with master is still required. No merge is authorized.
|
||||
|
||||
|
||||
## Security follow-up: individual refreshes
|
||||
|
||||
The company-hint design amplified ordinary connection writes into full company
|
||||
reads. The current contract scopes individual producers (API/example creation,
|
||||
setup/rollback, OAuth, gateway, health/catalog, email, voice, GitHub events, access
|
||||
removal, and Smoke Lab) by company plus connection ID. Existing application
|
||||
updates in example setup and Smoke Lab use application scopes. Company deletion
|
||||
and instance feature-settings changes retain broad invalidation. Startup and
|
||||
broad loads use bounded UUID pages. The health owner still defines eligibility
|
||||
and provider policy; the shared notification/coordinator layers remain generic.
|
||||
|
||||
See `doc/idle-tool-connection-health.md` for the failing amplification baseline,
|
||||
changed contract, scope-cancellation tests, and verification results. The older
|
||||
company-hint observations in this audit describe the superseded implementation.
|
||||
|
||||
## Acceptance follow-up after the plugin parent merged (October 27)
|
||||
|
||||
The actual merged parent is master `96727b3cfefb61f85ffe1fbc29a9b6dd6837efe8`.
|
||||
All 18 preceding sleep PRs are merged. Historical unmerged-state statements
|
||||
above describe earlier revisions.
|
||||
|
||||
The final producer and retry follow-up is recorded in
|
||||
[the health lifecycle document](../idle-tool-connection-health.md#acceptance-follow-up-after-the-plugin-parent-merged-october-27).
|
||||
It preserves the two real installer amplification failures, type-change lock-wait
|
||||
mutation proof, independent failed-inventory retry regression, fresh database
|
||||
fixture setup, and verification scope. Metadata-only installer writes do not
|
||||
invalidate an entire application; actual eligibility changes still do. Inventory
|
||||
failures do not inherit another company's one-second backlog continuation.
|
||||
Current-head reviews and CI remain required; these are worker-level guarantees.
|
||||
@@ -0,0 +1,31 @@
|
||||
# Configuration worker drain repairs
|
||||
|
||||
The voice drain audit exposed a pattern in three earlier independent workers:
|
||||
retained dirty scopes reported as pending work keep the coordinator's intent
|
||||
activity hold alive after the write has settled. No recurring SQL is necessary
|
||||
to block idle admission this way. The earlier tests checked traffic but did not
|
||||
check the idle counter or generation while paused.
|
||||
|
||||
Each owner must retain its hints and next deadline for resume, report dirty work
|
||||
as pending only while dispatch is allowed, and leave unresolved transaction
|
||||
fencing to the shared coordinator. No hint is acknowledged or discarded during
|
||||
drain. Business health/registration/lease deadlines and provider behavior stay
|
||||
unchanged. This is an idle-work boundary repair, not provider sleep qualification.
|
||||
|
||||
## Tool health — slice 18 / #17554
|
||||
|
||||
Baseline 9ef6e4f4c8 reproduces the bug: the strengthened existing drain case fails
|
||||
with active=1 after commit, recorded in /tmp/sleep18-drain-before.log. The repaired
|
||||
case asserts active=0, then a stable idle snapshot and no SQL/provider calls for a
|
||||
minute, then the saved work runs on resume without another producer.
|
||||
|
||||
All 23 lifecycle cases pass in /tmp/sleep18-drain-tests.log; production server
|
||||
typecheck passes in /tmp/sleep18-drain-typecheck.log. The test database is the
|
||||
fresh migrated paperclip_sleep18_drain_20261025_test. No test failures were retried
|
||||
blindly. Current master bf9a8551c3 has no changes to these three worker owners
|
||||
since the preceding aec86c794b inspection; full stack reconciliation remains open.
|
||||
|
||||
Discord command registration and receiver discovery require the same causal
|
||||
check at their own parents, then propagation through every dependent stack PR.
|
||||
The full original recurring-work inventory, milestone producer audit, older E2E
|
||||
failure and inbound cleanup/deletion policy finding remain outstanding.
|
||||
Reference in new issue
Block a user