3 Commits

Include renames

Author	SHA1	Message	Date
DottaandPaperclip	2c17d3ca52	fix(github): recover webhook verification in attention state (#16263) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review bots require signed webhook delivery before activation. > - Cloud can deduplicate an old ping after a connection repair. > - A fresh event could verify active connections, but an attention-state connection discarded it before authentication. > - This pull request allows signed delivery verification while the connection needs attention. > - It discards ordinary work until the manager completes the existing reconnect and activation checks. ## Linked Issues or Issue Description Refs #16259. Related: #16138 addresses the separate classification of local authorization failures. **What happened?** Live verification of #16259 found that a GitHub App already marked as needing attention remained stuck waiting for a signed ping. The Cloud relay had already delivered that ping. New signed events were discarded by the attention-state ingress gate before they could verify delivery. **Expected behavior** A fresh valid event from the bound installation verifies delivery. It does not activate the connection, change repository access, or start work. The manager can then complete the normal reconnect flow. **Steps to reproduce** 1. Put an existing App connection into attention state with webhook verification cleared after repair. 2. Deliver a fresh signed comment event for its bound installation. 3. Continue setup. Before this fix, webhook verification remains missing and reconnect cannot proceed. ## What Changed - Use a content-free verification path for signed ordinary GitHub events while the connection needs attention. - Allow a fresh signed event from the same installation to verify webhook delivery. - Verify delivery even from disabled or unavailable repositories. Discard the event before content storage, resource reconciliation, or runtime creation. - Keep newly verified connections in the provider-setup step. Later installation events cannot activate them before reconnect. Preserve normal installation recovery for existing qualified connections. - Extend recovery regressions to cover attention state, foreign installations, invalid signatures, missing installation IDs, unchanged resources, no wakeups, and successful reconnect after verification. - Document the recovery behavior. ## Verification - The targeted regression reproduced the attention-state failure before the change. - All ten recovery cases pass after the change. The tests cover disabled and unavailable repositories, content-free verification, and installation events after verification. - Server typecheck passed. - All 260 GitHub integration tests passed after the review fixes. Final server typecheck also passed. - `pnpm -r typecheck` and `pnpm build` passed. - The full local suite is running. One unrelated workspace-runtime conflict-reference test failed and then passed on its isolated rerun. Final totals and CI/review outcomes will be updated before merge. - Live verification will use the existing advisory review bot after deployment. ## Risks - Attention-state ordinary events authenticate under the current credential lease without creating a durable content record. They return before resource reconciliation or runtime creation. - Verification is still bound to the current credential lease, runtime generation, valid signature, and installation ID. It does not activate the bot or replace normal setup checks. - Paused, revoked, archived, and unconfigured connections retain their existing ingress rules. No schema changes or permission expansion. ## Model Used OpenAI Codex (GPT-6), with code inspection, editing, test execution, and browser verification. 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 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>	2026-10-14 05:23:42 +00:00
DottaandPaperclip	a8b652b085	fix(github): preserve review delivery through reconnect (#16259) ... ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - GitHub review bots turn authorized repository events into agent tasks and publish the result on the pull request. > - Cloud Apps send callbacks through a relay, but the direct callback recovery worker reported that URL as broken. > - Reconnecting the same App then prevented a new authorized publication from updating its existing working comment. > - Report-only runs could also produce an action-required check when no complete assessment was available. > - This pull request preserves comment identity across reconnects, verifies fresh signed delivery after repair, and keeps report-only results neutral. > - Users can receive the review without changing repository access or making an advisory bot block their pull request. ## Linked Issues or Issue Description **What happened?** A review could be saved in Paperclip while GitHub kept showing “Working on this…”. A same-App reconnect changed the runtime generation and the existing response receipt then failed with “GitHub response binding changed”. Cloud callback recovery also compared the relay URL with a direct tenant URL. An incomplete or failed report-only run could produce an action-required check. **Expected behavior** A fresh authorized attempt updates the same App-owned comment after reconnect. Stale attempts and revoked people remain denied. Cloud relay delivery does not produce a direct-callback mismatch. Report-only checks remain neutral and explain incomplete work honestly. **Steps to reproduce** 1. Accept a GitHub request and publish its working comment. 2. Reconnect the same App before the agent publishes its result. 3. Submit the result and inspect the working comment and check. 4. Separately, let a report-only run end without an assessment and inspect its check conclusion. Related: #16138 addresses the separate classification of local authorization errors as provider credential failures. This change fixes the response binding and report-only paths themselves. ## What Changed - Bind durable response comments to the App and agent. Keep per-attempt runtime generation checks and provider comment ownership checks, including legacy receipts. - Exclude Cloud-managed Apps from the direct webhook recovery scan and its stale callback-mismatch warning. The Cloud inbox owns relay retries. - Let a fresh authenticated event from the bound installation verify webhook delivery after repair. Keep signature, runtime epoch, installation, and credential lease checks. - Keep report-only assessment and terminal-run checks neutral. Preserve incomplete/error text and gated-review behavior. - Add integration regressions for reconnect races, legacy comments, failed runs, Cloud recovery, and foreign or forged webhook traffic. Update connector documentation. - Correct the assigned-tool search guidance to name the actual input fields (`offset` and `schemaOffset`) and show valid arguments. The failed automatic review had an input-schema rejection before tool execution; this clarifies recovery without expanding tool access. ## Verification - `pnpm -r typecheck` passed. - GitHub policy and integration suite: 291 tests passed (`vitest run server/src/services/chat-github-review-policy.test.ts server/src/__tests__/chat-channels.integration.test.ts -t 'GitHub|github'`). - The report-only policy regression failed before the fix and passed after it. - `pnpm build` passed. - Assigned tool discovery: 24 tests passed. Its guidance now maps returned pagination offsets to the correct input fields and tells the model to omit unused optional fields. - Full `pnpm test:run` is running. It encountered a 15-second timeout in the unrelated authenticated Workspace calendar-write test. That test passed on one isolated rerun. - After the review-requested module extraction, server typecheck and both direct/Cloud recovery activity integration checks passed. - All 55 non-skipped CI checks passed for `32e690279d847e44a6946b6c7cfb110fb11aff79`. Greptile scored that commit 5/5 with no unresolved findings. Live staging publication verification follows deployment. ## Risks - A durable response can now survive a same-App reconnect. Fresh authorization, runtime generation checks for each publication attempt, App ownership, and the publication lease still fence writes. - Report-only failures intentionally become neutral. The check still reports the failure; it does not claim a complete assessment. - Cloud-managed callbacks rely on their existing durable relay inbox. The direct callback recovery worker remains unchanged for direct installations. - No schema migration or permission expansion. ## Model Used OpenAI Codex (GPT-6), with code inspection, editing, test execution, and browser verification. 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 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 22:24:41 -05:00
DottaandPaperclip	ba00e9ef1a	refactor(server): isolate provider webhook handling (#16183) ... ## Thinking Path > - Paperclip connects provider messages to governed agent work. > - Webhooks must authenticate and persist messages before acknowledging them. > - The shared service still contains GitHub and Slack protocol details. > - The platform-boundary rule places those details in provider folders. > - This pull request extracts provider webhook handling behind typed methods. > - The service retains dispatch, runtime selection, and durable lifecycle recording. ## Linked Issues or Issue Description Depends on #16180; review this PR against its parent branch. Refs #16080. Related open work: #16163 changes GitHub automatic-author policy. This extraction preserves that policy implementation. **What existing behavior does this improve?** The ownership and readability of GitHub and Slack webhook handling. **Current behavior** The shared service implements GitHub signatures, replay fences, automatic event messages, and Slack callback verification. **Proposed behavior** Place those details in the respective provider folders and keep the common dispatcher small. **Reason and benefit** Provider code can be reviewed without reading the shared task service. ## What Changed - Extract GitHub request-local deadline and replay scope (60 lines). - Extract GitHub authentication and durable-ingress coordination (280 lines). - Extract GitHub event admission, message construction, and policy-skip receipts (140 lines). - Extract Slack workspace validation and authenticated callback observation (131 lines). - Keep the same service-owned background task set and GitHub provenance maps. Forward late-initialized callbacks without reading them during construction. - Reduce chat-channels.ts from 7,383 to 6,880 lines: 503 net lines removed. ## Verification - Merge preparation: replayed this stack onto current master after #16180 landed. Preserve master's new Telegram scheduling methods and connection-scoped Photon notifications. Server typecheck, callback syntax audit, and module-boundary check passed. Fresh integration, CI, and current-head review are running. Earlier verification below describes the previous stack commits. - Parent baseline: 544 selected GitHub, Slack, and webhook integration cases passed; 751 other cases excluded. - Initial extracted run: 543 passed and one idle-queue scan assertion failed. The idle-queue case passed separately on both the parent and extracted code. Its publication implementation is unchanged. The full selected-suite retry passed all 544 cases; the initial failure remains documented and its cause is not established. - Server typecheck, module-boundary check, and git diff --check passed. - Syntax-tree comparison verifies unchanged request-scope logic, durable ingress, leased authentication, event handling, workspace validation, and callback recording. All unrelated service functions remain unchanged. - Repository typecheck and production build passed for the parent extraction. Full-stack verification will run at the final handoff. The first broad local run hit a macOS PostgreSQL shared-memory fixture limit and was interrupted after that failure. - All 55 current-head CI checks passed. Greptile: 5/5 on f74ecb5094, with zero inline findings. No live third-party execution is claimed. - Previous stack validation on #16192 (55417debfe): repository typecheck and build passed; all 1,295 chat integration cases plus 14 source-wiring checks passed locally (1,309 total). All five PRs have current-head 5/5 reviews and 55 passing CI checks. The broad local pnpm test:run hit PostgreSQL shared-memory fixture failures and was interrupted after 868 seconds (exit 130); it is not a local full-suite pass. ## Risks Moving callback code can change initialization timing or lose request-scoped state. The new factories retain live option getters, the exact runtime context and provenance maps, and typed forwarding for late initialization. Authentication stays inside the existing credential lease. The common lifecycle write still completes before returning provider success. ## Model Used OpenAI GPT-6 via Codex, with code execution and repository tools. The precise serving variant 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 - [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 11:02:19 -05:00