Commit Graph
2 Commits
Author SHA1 Message Date
DottaandPaperclip 33ca62f776 Keep external-agent feature reads on their admission transaction (#19750)
## Thinking Path

> - Paperclip manages work for AI agents.
> - External native agents must pass current feature and authority
checks.
> - The broker checks feature settings inside its admission transaction.
> - The old feature reader uses the root database pool.
> - Two active transactions can leave no connection for that read.
> - This change uses the admitted transaction for the same policy read.
> - It keeps the existing company, principal and controller checks.

## Linked Issues or Issue Description

Refs: #19340. That merged change fixes retained native Retry ownership.
This change leaves that recovery contract intact. Related: #18744 and
#17833 fix caller-owned transaction reads in other domains. Root
searched live GitHub for open externalBotEnabled changes and found no
duplicate.

**What happened?**

An external-agent admission can wait for another pool connection while
it holds a row lock. A second transaction can occupy the remaining
connection and wait for that lock. The feature read must use the
transaction that already owns admission.

**Expected behavior**

Dot, Grok and Muse read current feature settings on the admitted
transaction. Disabled providers remain disabled. Current authority
checks remain in force.

**Steps to reproduce**

1. Use a real PostgreSQL pool with two connections.
2. Lock the settings row in the admission transaction.
3. Occupy the second connection with a transaction that waits for that
row.
4. Read the existing provider feature policy through the admitted owner.
5. Require the read to finish before the competing lock is acquired.

**Paperclip version or commit**

The fix was tested at `d8dece250b75cffee67d3b7f4f0e669826282164`, based
on master `839d41aabbc6d042250548aedcaa5c50d369dca3`. It now includes
master `131b566ca4fa866c4348739c75cfe48c805b3398` through normal merge
`c2031b0fb3e5d1b34ebff1d5af8cc050d129bd9a`. All seven fix files retain
their tested blobs and modes.

## What Changed

- Pass the admitted select owner through the broker identity contract.
- Use that owner for Dot, Grok and Muse feature settings.
- Add a read-only settings projection that reuses the existing managed
and staging view. It does not create the settings row.
- Add six real PostgreSQL controls for enabled and disabled providers.
- Add three broker-path PostgreSQL regressions for exact transaction
forwarding through Dot, Grok and Muse.
- Keep all newer master broker changes and existing authority checks.

## Verification

Local controlled regression passed at tested revision
`d8dece250b75cffee67d3b7f4f0e669826282164`:

- Baseline: restore only the five original master production leaves.
Keep the same read-only settings projection and regression file. One
enabled-Dot case failed with the intended `57014`; it still read the
correct `true` policy. Five cases were unselected.
- The trace held the admission lock at 2.441 ms. The second connection
waited for that lock at 9.494 ms. The root-pool feature read finished at
2015.585 ms, after the competing statement timed out at 2014.982 ms.
- Candidate: all six enabled/disabled Dot, Grok and Muse controls
passed, with zero skips, in 14.528 s. Each read returned the configured
policy before the competing lock was acquired. Every competing
transaction committed without an error.
- Fresh normal candidate production declarations passed in 17.080 s. The
complete 615-root owning test program passed in 4.756 s. Own dependency
setup and TypeScript export builds had passed before these controls.
- Known worker groups and sockets closed. The single post-owner audit
found no other sessions in the test database. No forensic database was
changed.

An earlier selector attempt skipped all six cases and opened no test
database connection. It is not a regression pass or failure. Source
inspection proved that Vitest 5 filters names with ` > ` separators,
while its JSON reporter joins names with spaces. The selected control
then used the unique title-only pattern; its separate baseline failure
is preserved above.

The later normal master merge preserves all seven tested files. The 46
incoming paths change Native/workspace recovery, types and presentation.
They do not change the direct feature/settings read graph. The
direct-policy evidence therefore carries from its tested revision. It
was **not rerun at the merged revision**. Fresh merged-head checks at
`c2031b0fb3e5d1b34ebff1d5af8cc050d129bd9a` passed: the content-aware
Shared/SDK producer (1.325 s), normal Runner core and surfaces
TypeScript programs (0.936 s and 0.693 s), and the full normal App graph
(28.933 s). The App check included all production layers and all four
test programs: 616 normal test roots and 397/409/412 tree roots. Known
build owners and sockets closed. Database/Eval and provider asset
outputs remained unchanged. No install, Cargo, binary build, provider
execution or PG test repeated. Fresh final-head remote CI and review
remain **pending**.

The test entry is
`server/src/modules/external-agents/feature-reader-owner.postgres.test.ts`.
It requires a dedicated external
`PAPERCLIP_FEATURE_OWNER_TEST_DATABASE_URL` ending in `_test` and an
owned `PAPERCLIP_FEATURE_OWNER_TRACE` path. Its normal migration hook,
two-connection pool, case deadlines and assertions remain unchanged. No
provider or Runner binary is needed for these controls.

The review also identified a wiring coverage gap: the six contention
controls call the readers directly. A new whole-three file now traverses
the actual Dot/Grok/Muse broker factories, normal pending-pairing
producers, dispatch and real execution-admission transaction.
Pass-through observations compare the exact admission transaction with
the owner supplied to the normal readonly settings projection. Each
provider is disabled through normal settings and must return real 403,
read actual false policy, leave the queued run without Native authority,
and create no assignment. No grant, ready binding, policy result or
successful authorization is mocked.

With only the original five master production leaves restored, all three
new cases failed at the exact missing-owner assertion (`readOwners`
expected length 1, actual 0) after passing the real 403 and
admission-scope checks. The full baseline had0 pass / 3 fail / 0 skip in
12.691 s. With the exact candidate leaves restored, the unchanged
whole-three file passed 3/3 with zero skips in 10.101 s. Both variants
used actual Node 24 / Vitest 5 forks and a real PG pool of two; all
known owners/groups/sockets closed. The candidate's private temporary
directory was removed; the failed baseline database/home remain
retained. Root independently read all three raw baseline failures,
rehashed source and outputs, verified known owner/group closure, then
observed no remaining test sessions. The successful database was
normally removed; the original failing database and home remain
retained.

The new test entry is
`server/src/modules/external-agents/broker-feature-owner.postgres.test.ts`,
using dedicated `PAPERCLIP_BROKER_FEATURE_OWNER_TEST_DATABASE_URL`. The
complete normal 617-root owning test program passed in 4.883 s after a
normal exported config refresh; prior production declarations and
unrelated configs remained byte-identical. No compiler or
dependency/Rust work repeated for the PG variants. These results were
obtained with the seven production/source files at c203 plus the new
test; they are not claimed as a new published-head remote CI result. The
review repair is committed at
`d043860eb93a26c4416f184697a212f027d538e2`; it adds only this 117-line
test file to `c2031b0f`. Tracked production bytes are unchanged. Exact
published head `d043860eb93a26c4416f184697a212f027d538e2` now passes 56
checks with four expected skips. Greptile reviewed this exact head at
5/5 and confirms that the broker-test gap is addressed. The complete
review thread is resolved. Ponytail stopped without a verdict; one
normal retry was requested and no active turn or new finding was
observed. No earlier-head result is used as a new-head pass.

The controls prove feature policy and transaction ownership. They do not
prove deployed suspension, hosted Dot/Muse eligibility or full Native
task execution. The separate private useful-case pass does not supply
missing current-master or accounting evidence.

## Risks

The broker identity contract changes for all three providers. A missed
adapter could use the wrong owner. Current semantic, real-policy and
exact broker-owner identity tests cover that error.

The read-only projection reuses the normal settings view. It preserves
managed and staging rules. An absent row is projected without a write;
the existing initializing getter stays unchanged.

No schema, public API, queue, timer, rate limit or provider deadline
changes. No historical Retry or physical-stop proof is changed.

## Model Used

OpenAI `gpt-6.1-sol` assisted with source changes and tool use. An
OpenAI GPT-6 family coordinator reviewed the work; its exact runtime ID
was not exposed. Context-window sizes were unavailable.

## Checklist

- [x] I have included a thinking path that traces from project context
to this change
- [x] I have specified the model used (with version and capability
details)
- [x] I have checked ROADMAP.md and confirmed this PR does not duplicate
planned core work
- [x] I have searched GitHub for duplicate or related PRs and linked
them above
- [x] I have either (a) linked existing issues with `Fixes: #` / `Closes
#` / `Refs #` OR (b) described the issue in-PR following the relevant
issue template
- [x] I have not referenced internal/instance-local Paperclip issues or
links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip`
URLs)
- [x] My branch name describes the change and contains no internal
Paperclip ticket id or instance-derived details
- [x] I have run tests locally and they pass (tested revision and
source-carry limits stated above)
- [x] I have added or updated tests where applicable
- [x] I have updated relevant documentation to reflect my changes (no
user-facing command or contract change)
- [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-11-07 01:02:03 -06:00
DottaandPaperclip b8ffeb973a feat(runner): add experimental Grok Bot agent participation (#16219)
## Thinking Path

> - Paperclip manages AI agents that perform work for a company.
> - The shared Runner owns task admission, authority, receipts and
finalization.
> - An existing Grok Bot needs to receive assignments and return results
through that Runner.
> - The Bot also needs governed access when it starts work from its own
conversation.
> - This PR adds agent-purpose MCP access and routine webhook dispatch.
> - It reuses the external-agent Rust Runner and keeps Grok capabilities
explicit.
> - The provider stays experimental until the real-client managed
journeys pass.

## Linked Issues or Issue Description

**Agent or provider**
Grok Bot, distinct from the Grok Build coding runtime in #13882.

**Why this adapter is useful**
Users can assign work to an existing Bot, receive progress, questions,
documents and files in Paperclip, and let the Bot initiate work under
its agent permissions.

**How the agent is invoked**
A webhook-only routine delivers bounded references. Dedicated
named-agent OAuth and MCP provide inbound calls. Setup follows [Grok
routines](https://cursor.com/help/grok-bot/routines) and [custom
plugins](https://cursor.com/help/grok-bot/connect-plugins). The current
desktop app needs its Bot-generated Webhook URL navigation link when
routine-row clicks fail.

The GitHub search found related Grok Build runtime work, but no existing
Grok Bot agent provider. This extends the roadmap’s existing agent,
Runner, governance and artifact foundations.

## What Changed

- Add default-off Grok Bot participation through the shared real Rust
external backend, preserving native provider identity guarantees and
distinguishing session closure from remote termination.
- Bind agent OAuth to company, agent, client, gateway and authorizer;
recheck permissions and task-bound execution authority without
manufacturing a native identity.
- Persist routine configuration, dispatch intents, authenticated
acknowledgement and recovery holds. Store URL/key together in encrypted
secret storage, use guarded public egress, and never automatically
replay ambiguous HTTP delivery.
- Reuse native tools, durable waiting, completion assessment and review.
Add task-bound file grants through existing artifact storage; suppress
proactive echo dispatch and synchronous result wake loops.
- Enforce the current native deliverable work-mode policy at file-ticket
issuance and use. Add operator retries for confirmed rejected wakes,
with retained receipts, stable retry intent and current-authority
checks. Recovery locks bindings before routine rows. Thread Stop
resolves the selected run and its agent.
- Cancel queued and preparing Stop targets before assignment creation,
restore saved setup after a failed request and successful retry, and
publish the Grok provider descriptor schema. Advance the versioned
Copilot source declaration for the shared schema change; its
qualification remains pending.
- Filter task summaries with the shared privacy policy, preserve
feedback admission when wake delivery is blocked, reconcile settled
delivery receipts without restoring work authority, and bind every Stop
control to its displayed run. Recovery records distinguish dispatch
uncertainty from a revoked session.
- Add resumable guided setup, credential replacement, consent,
management, observed work status and stop/recovery controls, plus an
interactive Storybook journey using the production task thread, question
form and report card. Grok’s own apps remain available; Paperclip
workspace and third-party tools are deferred.

## Verification

- Five focused protocol, authorization, person MCP and Runner files: 192
tests pass. The Grok assigned-work case executes the actual Rust Runner,
saves a document, uploads/deduplicates a file and reaches ordinary
finalization.
- Proactive intake/deduplication and renewed-session recovery pass.
These use scripted clients and synthetic storage; they are not
real-provider acceptance.
- Repository typecheck, production build, token gates, map checks and
Storybook build/catalog checks pass. The local full-suite campaign was
stopped after macOS shared-memory exhaustion repeatedly prevented
embedded PostgreSQL startup. No local aggregate pass is claimed. The
first clean-worker CI run identified avatar fixture/configuration, flag
defaults, feature-card ordering and OpenAPI route-coverage gaps; all
four focused regressions pass after correction. Those regressions pass.
The latest clean-worker CI run also found a Copilot source-hash
declaration mismatch after the shared descriptor update; the corrected
declaration passes all 28 integrity tests. The next review corrections
pass 144 rendered UI tests, 21 OpenAPI tests and 82 Runner
contract/integrity/history tests. The latest database fixtures were
blocked at startup by host shared-memory exhaustion; clean-worker CI
must execute their new regressions. Full current-head CI now passes: 54
successful checks and four intentional skips. The database regression
uses the PostgreSQL driver result shape; file-policy, rejected-feedback
and lock-order cases pass on clean workers. Current-head Greptile is 5/5
with no unresolved findings.
- Rendered setup steps and outbound-only verification were walked
through in Storybook. Simulation cannot enable Done without an
authenticated-reply state.
- Actual client OAuth, continued access, reconnect and one routine wake
with an exact chat acknowledgement were observed separately. This
candidate’s live authenticated challenge, assigned/proactive workflows,
Linux managed artifact, restart and cleanup are not yet qualified. The
provider stays disabled by default.
- The separate Cloud ingress change forwards only exact task-bound
upload/download paths and preserves host identity and header stripping.
- Current Cloud policy reserves Test for pre-merge app/harness
experiments; Staging accepts only master-derived revisions. Real-client
qualification needs a supported, authorized deployment target and
remains outstanding.

## Risks

One append-only migration adds routine/dispatch state and extends shared
bindings and file tickets. The unshipped migrations were regenerated
after upstream consumed their original numbers. Rolling back code
retains the durable rows; revoke grants and inspect unknown remote
activity before resuming dispatch. Grok cannot confirm remote
termination, stream its complete transcript or report spending. Plugins
are account-wide and do not isolate credentials between Bots.
Real-client managed acceptance remains required before enabling the
experimental provider.

## Model Used

OpenAI Codex based on GPT-6, with reasoning, code execution and
browser/native UI tools. The exact deployment model ID and
context-window size are not exposed by this runtime.

## 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-13 20:57:42 -05:00