Commit Graph
2 Commits
Author SHA1 Message Date
DottaandPaperclip 3a213ab987 refactor(chat): extract resource reconciliation and availability (#15834)
## Thinking Path

> - Paperclip manages AI agents and their work.
> - Chat resources control which provider destinations can admit that
work.
> - Inventory updates and lifecycle callbacks must preserve company
scope and operator choices.
> - These rules share a large service with runtime and credential
ownership.
> - This PR extracts resource reconciliation and its availability
transaction.
> - Reviewers can inspect resource changes together while the service
retains runtime control.

## Linked Issues or Issue Description

Refs #15823 (merged). Slice 2 of 5 now targets `master`. Remaining
order: #15834 → #15836 → #15844 → #15845.

## What Changed

- Extract shared resource inventory, audited selection and availability
settlement from the large service.
- Put GitHub repository parsing, rename/transfer repair, webhook
recovery and listing in `github/resources.ts`.
- Put Telegram group migration in `telegram/resources.ts`, using the
coordinator's existing transaction and timestamp.
- Put Slack labels, Teams IDs and Photon participant parsing in their
channel folders.
- Keep lease assertions, delivery settlement and the resource upsert
between Telegram migration steps in the shared coordinator.
- The shared module is 598 lines; provider resource modules total 409
lines. Update the dated plan with this reassessment.

## Verification

- On updated master, 15 existing resource integration cases and one
Telegram migration case passed before this folder move. The same 16
cases passed after. Other cases were excluded by the filter.
- Eight moved declarations and three transaction blocks match the prior
implementation. The GitHub provider guard remains in the caller.
- Server TypeScript, module-boundary and whitespace checks passed.
Rebuilt plugin SDK types after rebasing onto current master.
- Historical browser shard 8 timed out on inbox retry; its unchanged
[rerun
passed](https://github.com/paperclipai/paperclip/actions/runs/38134430238/job/114457529315).
[Original
failure](https://github.com/paperclipai/paperclip/actions/runs/38134430238/job/114455332214).
- Tests use local disposable databases and fixture provider transports.
No live third-party execution is claimed.

- Fresh Greptile review is 5/5 on
`e914f532ca66c429125492c1723df38afc6e13d7`, with a successful check and
no open review threads.
- Final-stack `pnpm -r typecheck` and `pnpm build` passed at
`57ec5ba94aecd860471f9b14db46f99120c7c790`.
- All CI checks passed on `e914f532ca66c429125492c1723df38afc6e13d7`.
- The final full-local-suite outcome for this cumulative stack is
recorded in #15845; do not infer that gate from the focused tests above.

## Risks

- Resource identity and migrations depend on lock and write order.
Provider modules use the original transaction; statement order and
credential fencing remain unchanged.
- Shared selection and availability still contain provider dispatch
predicates. They coordinate common state transitions and should remain
shared.
- Full local tests and current-revision CI remain required for a green
handoff.

## Model Used

OpenAI Codex, GPT-6 family, with reasoning and repository tools. The
exact runtime model ID and context-window size are not exposed in this
session. No separate agent was used.

## 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
- [ ] 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-12 09:39:51 -05:00
DottaandPaperclip fb21a3f30a refactor(chat): extract provider message normalization (#15823)
## Thinking Path

> - Paperclip manages AI agents and keeps their work attached to company
tasks.
> - Chat providers deliver messages and corrections into those tasks.
> - The chat channel service has grown to 39,155 lines.
> - Provider message normalization has no database or worker ownership
of its own.
> - This pull request moves that responsibility into one module while
preserving the service body.
> - Reviewers can now find provider identities, timestamps, and edit
revisions together.

## Linked Issues or Issue Description

**What existing behavior does this improve?**

Maintenance of inbound chat message normalization in the server.

**Subsystem affected**

Server chat channels.

**Current behavior**

Provider message parsers and identity rules share a 39,155-line file
with credentials, runtime ownership, database admission, and publication
workers.

**Proposed behavior**

Move provider message normalization into
`chat-channels/provider-messages.ts`. Keep service exports and runtime
behavior unchanged.

**Reason and benefit**

This creates a clear first boundary for an incremental refactor. A dated
plan ranks the next four larger extractions. No duplicate chat refactor
was found in the GitHub title search.

**Breaking changes**

None intended. No schema, endpoint, transport, or public service API
changes.

Stack order: #15823 (provider messages) → #15834 (resource
reconciliation) → #15836 (identity links) → #15844 (publication
replacement) → #15845 (failed-run retries). Merge in this order;
retarget each child after its predecessor lands.

## What Changed

- Move provider message parsing into
`chat-channels/telegram/messages.ts`, `slack/messages.ts`,
`discord/messages.ts`, `microsoft-teams/messages.ts` and
`github/messages.ts`.
- Keep shared actor normalization, actor data and the inbound text limit
in `chat-channels/message-lifecycle.ts`. These six modules total 566
lines / 19,534 bytes.
- Import provider modules directly. Follow this provider-folder
convention for future extractions.
- Preserve exact actor IDs, edit hashes, timestamp provenance, and
Telegram message identity rules.
- Narrow two input types to the Teams parser method and the retained
delivery fields actually read.
- Reduce `chat-channels.ts` from 39,155 lines / 1,509,090 bytes to
38,643 lines / 1,491,703 bytes.
- Record five ranked extraction candidates and verification evidence in
the dated plan.

## Verification

- The original extraction passed complete CI and 5/5 review at
`cbba30a526`. Those results are historical; the updated head is
`90ac517e6c310c8cf194dbbd668006f740e58f3a`.
- Structural comparison: all 21 moved declarations match the base after
normalizing exports and two type-only changes. All remaining non-import
declarations, including the complete service body, match the base.
- Provider-folder update: the same 48 public service integration cases
passed locally before and after the move on PR #15823. They cover
Slack/Discord revision identity, Teams edits, GitHub edits, Telegram
rich messages and provider chronology.
- All 22 message declarations retain identical bodies; the service
changes imports only. The four dependent extractions rebased without
conflicts.
- Final-stack workspace `pnpm -r typecheck` passed. Module-boundary and
whitespace checks passed.
- Final-stack `pnpm build` passed. The fresh `pnpm test:run` was
interrupted after PostgreSQL startup failures recurred. An isolated
`native-status-arbiter-corpus.test.ts` run failed during database
initialization after five attempts; all 45 tests skipped. All 32 host
shared-memory slots were occupied. The full local gate remains
unverified; only the task-owned run was stopped.
- Greptile rated updated head `90ac517e6c310c8cf194dbbd668006f740e58f3a`
5/5 with no inline review threads.
- Updated-head CI passed: 53 successful checks and two skipped checks,
including all chat integration shards, typecheck, build and browser
suites.
- Tests use disposable databases and fixture provider transports. No
live third-party execution is claimed.

## Risks

- A wrong module import or changed revision identity could affect
message admission. Existing service-path tests and the declaration
comparison cover those risks.
- Authentication, company-scoped transactions, leases, background
workers, and shutdown remain in the original service.
- The service closure remains large. Resource reconciliation is the
recommended next extraction.

## Model Used

OpenAI Codex, GPT-6 family, with reasoning, local code execution, and
repository tools. The exact runtime model ID and context-window size are
not exposed in this session. No separate planning or review agent was
used.

## 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
- [ ] 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-12 08:21:46 -05:00