mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 01:41:42 +02:00
DottaandPaperclip
14afc12124
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Coding agents extend connectors and shared control-plane services. > - Provider details can leak into shared services during these changes. > - Moving code into a new file does not explain why shared behavior must change. > - This pull request adds module ownership and shared-change review rules. > - The rules help agents justify the change before implementation and review. ## Linked Issues or Issue Description **Issue type** Missing documentation. **Where is the issue?** Repository architecture rules and the connector authoring playbook. **What's wrong?** Subsystem rules describe some ownership boundaries. The repository lacks a general rule for deciding which behavior belongs in a feature module, what shared contracts must change, and how to review the resulting scope. An integration can therefore add provider-specific logic to shared code or bundle large platform changes into one connector PR. **Suggested fix** Add a repository-wide rule and load it in Greptile. Apply the rule during connector planning and PR preparation. Keep provider protocol behavior with its owner. Require a reason for shared changes and checks through affected consumers. Related public context: [Notebook connector PR #16984](https://github.com/paperclipai/paperclip/pull/16984). This PR changes instructions only. ## What Changed - Add a concise module-boundary rule for ownership, narrow contracts, shared controls, delivery scope, and verification. - Register the rule as repository-wide Greptile context. - Add planning and PR review steps to the connector playbook. Keep tests and docs with their behavior. ## Verification - Passed rule frontmatter, Greptile registration, referenced-file, and playbook-link checks. - Passed `git diff --check origin/master...HEAD`. - Reviewed the rule against the relevant connector failure and allowed cases: small dispatch branches, one-consumer shared capabilities, and unrelated fixes. - Fresh Greptile and Paperclip reviews both scored commit `759572fe503e1b6dfa9527f53de6c326a2c3bff6` 5/5. No actionable findings or unresolved review threads remain. - Final current-commit GitHub checks: 55 successful, 1 neutral, and 2 skipped; no pending or failed checks. Both required `ci / verify` and `ci / e2e` gates passed. - Initial CI attempt: the build and runtime boot smoke test passed. The spot runner received a shutdown signal during the clean public npm install check, canceling that step and failing the build job. [Interrupted job log](https://github.com/paperclipai/paperclip/actions/runs/39601811838/job/119228042468). Retried failed jobs once. Attempt 2 passed the complete build, runtime boot, and clean-install verification. The initial runner interruption remains recorded here. - Full local application checks are unavailable: this checkout has no workspace dependencies, and `pnpm -r typecheck` stopped during pnpm bootstrap because registry fetches failed and the requested pnpm version could not be verified. Local application tests and build were not run. GitHub checks must pass before merge. ## Risks - The rule could cause unnecessary abstractions or extra PRs if applied mechanically. It permits small dispatch branches and useful capabilities with one consumer. It rejects arbitrary file counts and unrelated restructuring. - Prose and metadata checks do not prove that future agents will follow the rule. Hosted review receives it when this branch is reviewed. ## Model Used - OpenAI Codex: `gpt-6-astra` authored the guidance; `gpt-6.1-sol` prepared delivery. Both used reasoning, filesystem tools, and GitHub tooling. Context window size was not recorded in the available model settings. ## 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 (focused documentation and metadata validation; application checks unavailable as stated above) - [x] I have added or updated tests where applicable (no runtime change; targeted structural checks cover the changed instructions) - [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>
47 lines
2.8 KiB
Plaintext
47 lines
2.8 KiB
Plaintext
---
|
|
description: Keep behavior with its owning module and justify changes to shared contracts and delivery scope.
|
|
alwaysApply: true
|
|
---
|
|
|
|
# Module boundaries
|
|
|
|
Apply to new or materially changed behavior across modules. Follow subsystem
|
|
ownership rules; unrelated code need not be reorganized.
|
|
|
|
- **Choose ownership first.** Identify the owner, existing contract, and missing
|
|
shared capability in the task's existing plan or PR. No separate document or
|
|
approval is required. Convenient local variables do not justify orchestrator logic.
|
|
- **Isolate implementation details.** Provider protocols, payload parsing,
|
|
workarounds, and feature sequencing belong with their owner. Shared code consumes
|
|
normalized domain contracts, not provider payloads or SDK types. Small registries
|
|
and dispatch branches may select implementations and delegate behavior.
|
|
- **Keep contracts narrow.** Reuse extension points; add only what the workflow
|
|
needs. Pass required dependencies, not the whole service context. Moving code
|
|
or renaming interfaces does not remove coupling. A coherent shared contract
|
|
should survive provider removal or substitution. One consumer can justify it;
|
|
speculative reuse or another fixture alone cannot. Preserve provider differences
|
|
rather than forcing a universal interface.
|
|
- **Preserve invariant ownership.** Authorization, company isolation, audit,
|
|
transactions, and resource ownership retain their existing owners. Adapters
|
|
must neither bypass controls nor create parallel enforcement paths.
|
|
- **Explain shared changes.** For each substantive change outside the owner,
|
|
explain the capability, why existing contracts are insufficient, affected
|
|
consumers, and behavior with the feature absent or disabled. Group related files.
|
|
Catalog entries, branding, schemas, and registration may stay in established
|
|
locations without teaching shared execution the provider protocol.
|
|
- **Separate ownership from delivery scope.** Even well-factored platform work
|
|
may need a separate reviewable slice or explicit PR-stack dependency to clarify
|
|
effects on existing consumers. Keep tests and docs with behavior. Preserve
|
|
requested features and delivery constraints; avoid unusable intermediate states
|
|
and arbitrary PR/file counts.
|
|
|
|
**Verify:** Exercise observable behavior through new and relevant existing
|
|
consumers, including affected absent/disabled states—not just helper calls.
|
|
A different-protocol fixture can expose coupling but proves neither production
|
|
compatibility nor the abstraction's necessity.
|
|
|
|
**Review:** Flag misplaced behavior, leaked private representations, duplicated
|
|
invariant ownership, and unexplained shared changes; name the proper owner and
|
|
contract. Recheck necessity and scope after refactoring. Identifier searches,
|
|
line counts, and passing CI alone do not establish sound boundaries.
|