mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 20:44:16 +02:00
DottaandPaperclip
13e5128164
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Its control plane shares execution capacity across requests and agent orchestration. > - A dependency can return a small value while doing slow or repeated work underneath. > - Tests can check that value while mocks hide the behavior that affects other work. > - A later passing retry can also leave an earlier material failure unexplained. > - This pull request records general rules for dependency work, verification, and acceptance. > - The benefit is a durable review habit that applies beyond the runner incident. ## Linked Issues or Issue Description Refs #15683. Refs #16335. The staging runner incident exposed a gap in how we establish the operational cost of a dependency call. The original platform check read a small field from a process report. The call could block shared execution, and value-focused mocks did not exercise that behavior. The fix replaced that probe with bounded asynchronous work. This proposal addresses the reasoning and evidence gap, without assigning blame or prescribing one API or timeout for future work. The acceptance guidance also preserves material failures after a retry passes. It does not assert that every earlier test failure had the same cause as this incident. Related proposal checked: #14275 covers release sign-off templates. This PR covers implementation assumptions and verification evidence. ## What Changed - Add a scoped rule for dependency work in server, adapter, and runner execution paths. Cover blocking, repetition, cost growth, cancellation, and the effect on unrelated work. - Require evidence that can disprove the consequential assumption introduced by a change. Identify behavior removed by mocks and exercise the relevant boundary. - Register the rule in the Greptile manifest with matching path scope. Existing `AGENTS.md` instructions route coding agents to matching rules. - Add failure preservation to the verification skill. Link it from the repository-wide PR review rule. Keep recovery and resolution distinct. - Include examples and exceptions so cosmetic edits and ordinary pure transformations do not acquire unnecessary test requirements. ## Verification - Passed `git diff --check` and the repository PR template and duplicate-search validators. - Passed a local Python structural check: manifest JSON parses; rule globs and Greptile scopes match; seven representative runtime paths match; five unrelated paths do not; the evidence link and heading resolve; existing agent and reviewer loading instructions are present. - Manually applied the rule to a network-filesystem configuration getter, a repeated remote permission check, and an already-loaded map lookup. The first two need evidence about their new dependency risk. The last does not require fault testing by default. - Attempted `pnpm -r typecheck`, `pnpm test:run`, and `pnpm build`. The installed pnpm launcher could not fetch registry data to verify the requested pnpm version. Using the cached project version reached typecheck, which failed because workspace dependencies are absent (`@types/node` missing). Tests and build remain unverified locally. - Greptile reviewed commit `e56d1ec70f3ebffe697b35cc637975b8d8198681` at 5/5 with no unresolved review threads. CI is still running. These structural checks establish instruction wiring, not future compliance by an agent or reviewer. ## Risks - The main risk is adding ceremony to low-risk edits. The rule limits requests to concrete risks introduced by changed behavior and explicitly exempts unrelated work. - Instructions depend on contributors and reviewers reading and applying them. This change adds no automated runtime regression gate. - No application behavior, schema, dependency, lockfile, or workflow changes. ## Model Used OpenAI Codex, GPT-6 family. The exact serving model ID, context window, and configured reasoning effort are not exposed in this session. Used reasoning, repository inspection, shell tools, and patch editing. ## 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 available in this session) - [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 (guidance-only change; structural validation described above) - [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>
55 lines
2.8 KiB
Plaintext
55 lines
2.8 KiB
Plaintext
---
|
|
description: Account for dependency cost on shared runtime paths and test the assumptions that mocks can hide.
|
|
globs: server/src/**,packages/adapter-utils/src/**,packages/adapters/*/src/**,packages/paperclip-runner/src/**,packages/paperclip-runner/runner/**
|
|
alwaysApply: false
|
|
---
|
|
|
|
# Runtime dependency boundaries
|
|
|
|
Apply this rule when adding or moving dependency calls into startup, request
|
|
handling, discovery, or recurring orchestration in the server, adapters, or
|
|
runner. A dependency includes a runtime API, filesystem, database, subprocess,
|
|
or remote service.
|
|
|
|
## Account for the work at the call site
|
|
|
|
Establish what the dependency actually does: whether it blocks shared execution,
|
|
how often it runs, what its cost grows with, and how it fails or stops. Reading a
|
|
small result does not imply a cheap operation. Use implementation, documentation,
|
|
or measurement as evidence when the behavior is not established.
|
|
|
|
Match repeated work to how often its underlying facts can change. Account for
|
|
calls inside loops and repeated requests. Keep unrelated work responsive when
|
|
the dependency is slow or unavailable. If using a timeout or cancellation,
|
|
verify that it can take effect while the operation is running and account for
|
|
work that continues after the caller stops waiting.
|
|
|
|
## Test the assumption that could invalidate the design
|
|
|
|
Identify the consequential assumption introduced by the change and choose
|
|
evidence that could disprove it. Check whether mocks remove the behavior being
|
|
relied upon, such as timing, contention, cancellation, persistence, or external
|
|
side effects. Where they do, exercise that boundary with a representative real
|
|
dependency or a controlled fault that preserves the relevant behavior.
|
|
|
|
Verify the affected contract, including unrelated work that must remain
|
|
responsive, rather than only the returned value. Keep coverage proportional to
|
|
the changed behavior; an exhaustive environment matrix is unnecessary. Record
|
|
the assumption, evidence, and remaining limits in the PR's verification section.
|
|
|
|
## Examples and limits
|
|
|
|
- A configuration getter that reads a network filesystem needs evidence about
|
|
slow or unavailable storage and the effect on other work using that process.
|
|
- A remote permission check moved inside a search-result loop needs evidence
|
|
about call count and latency as the result set grows. Any reuse must preserve
|
|
authorization scope and freshness.
|
|
- Reading an already-loaded map or changing a pure value transformation does
|
|
not require dependency-failure testing unless the change introduces a
|
|
consequential cost or behavior assumption. Cosmetic edits do not trigger it.
|
|
|
|
**Review:** Check the changed call sites, the operational assumption, and whether
|
|
the supplied evidence exercises it. Request missing evidence for a concrete risk
|
|
introduced by the change. Do not require unrelated refactors or a new benchmark
|
|
suite for every edit.
|